-
Notifications
You must be signed in to change notification settings - Fork 5
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
Bug fix related to issue #131, used wrong variable in calculation of … #132
base: master
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.
Changes look good and well confined, I'll hold merging until @MatthieuSchaller confirms this fixes the problem, but I'll approved now.
I have tried it and it hasn't crashed but the code timed out before completion. |
This seems to have worked. I am crashing when writing the catalogs now but that is another problem I reckon. I am checking with more nodes just to confirm. |
Actually, crashed again. 32 ranks with 32 threads each. 752 baryon particles. 4x more DM.
stderr:
|
I'll see if I forgot something when pulling out some minimal changes. |
Would you know of a version SHA that pre-dates the MPI refactoring but provides the same physics features as the latest version? |
Hi @MatthieuSchaller , just a quick note that I'll look for the SHA with older MPI code but also I will only be able to test large stuff in two days or so when Setonix is back online. I think the issue is I left some variables as ints and should be unsigned ints and also others needed to be long long int (for offsets in the buffer). |
Shall I test these latest changes? |
@MatthieuSchaller , you're welcome to give it a try but I haven't tried them yet either. I just figured there must be issues with ints, and these updates should correct stuff to using unsigned ints and long long ints when appropriate. I've submitted a few tests to run on some large and small sims and should have results to look at tomorrow. It is possible that stuff might crash but hopefully I haven't forgotten anything. |
Hi @MatthieuSchaller , I tried with a dm 2600^3 sim, 48 mpi ranks, each rank having ~3-4e8 particles and it ran without issues. Did the updates work for you? |
... offsets sending data.
This is to address issue #131. Matthieu can try this fix.