-
Notifications
You must be signed in to change notification settings - Fork 13
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
New slicing backend (nearest neighbors interpolation) for use with moving mesh #182
Conversation
Amazing! Love it. Thank you for this contribution; we should definitely include this. Thank you also for so carefully working within the existing framework. |
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.
Some minor comments, but looks good.
Thanks for the comments! I resolved most of them and left some questions. |
That should be it then, I think? |
Looks good. We will need to release a new version (v8.0.0) as this contains api breaking changes. |
I just pushed another commit in which I adjusted the spelling of "neighbors" to "neighbours" (like it was already used in |
It turns out |
Code needs to be formatted. |
Not required for this PR, but you could imagine actually not even needing a tree for this. For densely populated areas, you can just find the particle nearest to the pixel center by looping through all the particles, and for the sparsely populated areas a flood fill after that first procedure would work... |
Yeah, I did consider that actually, as it will probably be faster, but ended up using the tree method, since I already used it outside swiftwimio and hence was very easy to implement. Something to keep in mind for later probably though. |
Thank you so much for this contribution! |
I implemented a new slicing backend (similar to how there are multiple backend for the projection) which uses nearest neighbor interpolation of the quantities as this is more suitable for moving mesh hydrodynamics.
All tests pass and I modified the relevant ones to also test the new backend. All features of the original slicing method (now still the default and dubbed
"sph"
) should be supported by the new"nearest_neighbors"
backend. No default behaviour has changed, so this should be fully backwards compatible.I also added a few sentences to the documentation.
Do you think this is suitable for including in
swiftsimIO
?Do you have any remarks/wishes?