Skip to content
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

Update particle splitting script for particles with >64 splits #61

Merged
merged 14 commits into from
Dec 11, 2024

Conversation

robjmcgibbon
Copy link
Collaborator

@robjmcgibbon robjmcgibbon commented Oct 14, 2024

When calculating which particles have split from other we make use of the SplitTrees dataset output by SWIFT. For Colibre the datatype is a long, so it can hold at most information about 64 particles splits. We were finding some particles that had more than 64 splits, and so couldn't reproduce the full split trees.

In https://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/2000 we are now logging information about particles that exceed this limit to a text file. This PR updates HBT to read in this new SWIFT output.

TODO:

  • Decide how to handle particles with >64 splits if no text file can be found
  • Test on the small low z EAGLE box

@robjmcgibbon robjmcgibbon marked this pull request as ready for review October 17, 2024 13:31
@robjmcgibbon
Copy link
Collaborator Author

Ready for review, I've tested this on the EAGLE_6 example, and everything seems to be working as expected.

@VictorForouhar
Copy link
Collaborator

Is this ready to be reviewed?

@robjmcgibbon
Copy link
Collaborator Author

Yep, I tested it on the EAGLE box by setting the size of SplitTree to 8 to cause overflows

@robjmcgibbon
Copy link
Collaborator Author

Maybe we should update the pipeline sbatch scripts to pass the particle splitting directory

@VictorForouhar
Copy link
Collaborator

Yes, we should update the pipeline scripts so that it defaults to loading the overflow split information. Safer that omitting it.

Copy link
Collaborator

@VictorForouhar VictorForouhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy for you to take the initiative to merge into master.

@robjmcgibbon robjmcgibbon merged commit bd974e4 into master Dec 11, 2024
@robjmcgibbon robjmcgibbon deleted the overflow_particle_split branch December 11, 2024 12:45
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.

2 participants