Repository navigation
Add Dynamic Shared Memory example. - #8839
Conversation
gmarkall
left a comment
There was a problem hiding this comment.
Many thanks for this! This is something I've been meaning to get round to for literally years but never quite managed it (instead answering the same question multiple times over the years!). I think this is looking generally great - I have made some suggestions on the diff, which are mostly about formatting and grammar / spelling, but a couple of more substantive suggestions too. In addition to the comments on the diff, the lines need wrapping at 80 characters as well - this isn't enforced by a style check in CI, but is the convention for the docs.
I have manually checked that the code snippets execute correctly and demonstrate the correct output. Ideally new code in the docs would be a doctest, but I'd suggest we just fix up the things referred to in the comments here and then could convert to a doctest in a future PR, so that we can get this over the line quickly.
Once you've made adjustments, please ping me and I can do another round of review!
|
@gmarkall all changes as suggested. Thanks for the review! |
gmarkall
left a comment
There was a problem hiding this comment.
Many thanks for the updates - there are a couple of other minor points that I missed last night, if you'd be OK to make those changes.
I didn't know about pycon code blocks, so that was a mistake on my part to suggest changing them to python - feel free to change them back to pycon if you'd prefer.
…into shared_mem_example
|
Changes committed. I've decided to use Thanks for the review @gmarkall. |
gmarkall
left a comment
There was a problem hiding this comment.
Looks great, many thanks for fixing this longstanding issue!
|
Yeehaa 1st Numba PR :) |
|
Closed it accidently. Should I keep this open and it'll auto-close when merged? |
|
Yeah, it will close when merged - the current state of things is all good. |
|
I've linked this to #5025 so it will close too - I think this is sufficient documentation for dynamic shared memory, so we might as well have one less issue on the tracker 🙂 |
@gmarkall have provided excellent demonstration of dynamic shared memory as answer to my question on numba discourse. It is essentially copy and paste of @gmarkall 's answer.
This PR partially addresses #5025.
Fixes #5025.