Skip to content

Conversation

@erikvansebille
Copy link
Member

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)

@erikvansebille erikvansebille requested a review from CKehl August 17, 2021 11:39
Copy link
Contributor

@CKehl CKehl left a 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.

@erikvansebille
Copy link
Member Author

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!

@erikvansebille erikvansebille merged commit 0b6f17d into master Mar 1, 2023
@erikvansebille erikvansebille deleted the speedup_C-contiguous_arrays branch March 1, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants