-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Speedup C-contiguous arrays #1074
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, just to add the final note to this PR:
For now, I highly discourage those changes of array-copy changes in the main branch (especially before wrapping up the node-list particle set), as it will invalidate any benchmarking already done, and hence make me redo any publishable measurements.
The changes made until now look quite good. I suggest pausing this PR and continuing with it as soon as the performance benchmarks can be easily separated from main again. At that point, the engineering improvements can safely be pursued without invalidating publishable measurements.
Sure, I understand and was not planning to merge this PR before you had completed your benchmarks. I was planning to use this branch to do some benchmarking of its speedup on of my own simulations. Let's then merge this PR asap (surely before the next Parcels release) so that users can take advantage of it too; would be a shame to deny them this potential speed-up for their simulations! |
This PR implements the 'engineering speedup' proposed by @CKehl in #957 by making sure data arrays are C-contiguous from the start (so that they don't need to be transposed on the fly)