-
Notifications
You must be signed in to change notification settings - Fork 100
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
Added collision handler for Huayno OK algorithm #1058
base: main
Are you sure you want to change the base?
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.
It seems like this is effectively the contents of the detect_collisions()
function from evolve_shared_collisions.c
, copy-pasted into evolve_ok.c
. This is unfortunately done quite frequently, but it's not good programming practice.
The point of functions is that they can be called from multiple places in the code, thus avoiding duplication. And that is important, because duplicates will diverge over time. Imagine that someone finds a bug in the original function and submits a fix. Will they know that there's a copy somewhere that should also be fixed?
Perhaps you can modify this to call that function, rather than copy-paste it? If you need to change the function, you'll have to carefully check where else it's called and make sure that those cases will still work as well, of course.
I made a new file which hosts the collision detection function and with which both evolve_shared_collisions.c and evolve_ok.c call. The algorithm neglects collisions between two massless particles to save computing time at the detriment that perhaps this isn't an assumption that some users would want. If that is a bother I can rewrite the code to include it. I hope the changes are fine. I tested collisions using either integrators and I don't see a change in performance. |
This PR needs at least a test to show that this collision detection actually works before it can be merged. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 28 days if no further activity occurs. Thank you for your contributions. |
Integrated the collision detection algorithm from the various SHARED_COLLISION Huayno integrators into the OK integrator.
Note: This collision checker does not detect collisions between two massless particles.