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

Limit number of lost particles written to disk. #2688

Merged
merged 17 commits into from
Sep 26, 2023

Conversation

ebknudsen
Copy link
Contributor

@ebknudsen ebknudsen commented Sep 8, 2023

Description

This PR adds an option to limit the number of lost particles written to disk. This can be useful in the admittedly rare corner case when debugging leaky geometry models, to avoid being overwhelmed by the sheer number of files.

No regression test has been added - I simply don't know how to reliably generate a simple leaky geometry.

Fixes # (issue)
No present issue

Checklist

  • [ x] I have performed a self-review of my own code
  • [ x] I have run clang-format on any C++ source files (if applicable)
  • [ x] I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@ebknudsen
Copy link
Contributor Author

I've now added a regression test - based on the procedure of particle_restart_eigval, which seems stable enough

@paulromano
Copy link
Contributor

@ebknudsen I've made some updates here on your branch:

  • I've changed the default limit to -1, which means that if a user sets it to 0, it will completely disable the writing of lost particle files (seemed like that might be a desirable option for some).
  • Fixed up associated documentation
  • Refactored the test you had into a smaller "unit test" (for this case, we really don't care about the result, only that the correct number of lost particle files was created).

Since this was a little bit of a heavy-handed approach of making changes rather than suggestions, let me know if it looks OK to you; happy to iterate further if needed!

@ebknudsen
Copy link
Contributor Author

@paulromano Cool. All fine by me. I agree on all your edits and btw. I have absolutely no problem with the direct edits :-).

@paulromano paulromano merged commit d765bd0 into openmc-dev:develop Sep 26, 2023
pshriwise pushed a commit to pshriwise/openmc that referenced this pull request Sep 27, 2023
@ebknudsen ebknudsen deleted the limit_write_lost branch October 11, 2023 10:44
stchaker pushed a commit to stchaker/openmc that referenced this pull request Oct 25, 2023
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
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