openmc icon indicating copy to clipboard operation
openmc copied to clipboard

Addition of a collision tracking feature

Open saliba170499 opened this issue 6 months ago • 4 comments

Greetings,

I added a collision tracking feature. the user can select parameters to select/filter for specific collision types. The information is saved in a collision_track.h5 (or .mcpl) file. The data can then be extracted via the python api.

The formatting of the C++ code was done using clang-format15. Python follows the PEP8 format. Documentation has been added. Unit and regression tests were added.

Looking forward to an eventual merging in the dev branch!

Thank you very much for your time for reviewing and your support,

Kind regards, Michel Saliba PhD at LRS - EPFL

saliba170499 avatar May 23 '25 13:05 saliba170499

Is this related to this paper:

Development of an Event Tracking Feature in OpenMC for Neutron Spectroscopy and Scatter Camera Systems

10:50–11:15AM MDT

Michel Saliba (École Polytechnique Fédérale de Lausanne), Paul Romano (ANL), John Tramm (ANL), Erik B. Knudsen (United Neux), Daniel Siefman (Univ. California, Berkeley), Andreas Pautz (Paul Scherrer Institute), Oskari Pakari (École Polytechnique Fédérale de Lausanne)

MicahGale avatar May 23 '25 15:05 MicahGale

@MicahGale - yes, this is the feature from that paper.

jtramm avatar Jun 10 '25 14:06 jtramm

Thanks for submitting this @saliba170499! Generally the PR is looking good -- code looking clean and should make for a useful feature!!

A few high level topics before I go through and make finer-grained comments:

  1. It looks like the C++ formatting checker is failing. I'm not sure what would be causing that failure, as you mentioned you had run v15 of clang-format already, but perhaps it is not finding the config file somehow. Try running it again on your end (or on another machine) to see if it makes more changes. For reference, the CI gives the following complaints:
  Notice: File include/openmc/mcpl_interface.h does not conform to Custom style guidelines. (lines 34, 43)
  Notice: File include/openmc/settings.h does not conform to Custom style guidelines. (lines 159, 160, 161)
  Notice: File include/openmc/state_point.h does not conform to Custom style guidelines. (lines 46, 53)
  Notice: File src/settings.cpp does not conform to Custom style guidelines. (lines 974)
  1. I'm noticing that some of the results_true.dat files just have k eigenvalue information in them, which doesn't seem useful for testing this feature, though perhaps the event files are tested using another strategy? It would be helpful if you could give some high level details on your testing strategy as part of the PR writeup (or a comment on the discussion here). Also, what is the purpose of all the different test cases (e.g., case-01, case-02, ..., case-a02?) If they are testing different types of conditions, might be useful to give them each a more descriptive name. For long term maintenance, if one breaks, it is nice to know right away what is broken based on the test name.

Not a big deal, but for future PRs, it is preferable to restrict auto formatter changes to only lines of code that you worked on. C++ files are usually safe to do globally as the clang-format program is very consistent, but the style guidelines for python aren't as well defined. Running it globally on python scripts generates a much larger diff which makes reviewing more time consuming/painful.

Anyway, thanks again for putting this in! I'll make a finer-grained pass once the tests are passing.

jtramm avatar Jun 10 '25 15:06 jtramm

Thank you very much for your review, @jtramm.

This all makes sense. I will address your points and provide a short outline of what the tests are probing for.

saliba170499 avatar Jun 11 '25 06:06 saliba170499

@jtramm ,I've been thinking about the testing strategy, and I agree the current structure could be clearer. I'm thinking of re-designing the tests like this:

Tests directly tied to user input parameters:

  • case_1_MT_number: Tests tracking collisions with only MT numbers set, like: 'settings.collision_track = { 'max_collisions': 300, 'mt_numbers': [2,18] }'
  • case_2_Cell_ID: Uses only the cell ID filter.
  • case_3_Material_ID: Checks material ID filtering alone.
  • case_4_Nuclide_ID: Tests with nuclide IDs as the only discriminator.
  • case_5_Universe_ID: Universe IDs only, to confirm this filter individually.
  • case_6_deposited_energy_threshold: Collision tracking with a deposited-energy threshold specified, verifying threshold logic.
  • case_7_all_parameters_used_together: A comprehensive “kitchen sink” test using every available input parameter simultaneously.

I agree that just looking at k-eff mean and standard deviation in results_true.dat doesn't strongly validate this particular feature. However, the main correctness check will always be comparing generated collision-track HDF5 files (collision_track.h5) against reference outputs (collision_track_true.h5). Those comparisons would be hopefully enough to catch any bugs introduced later.

Additional tests (already included in the current PR):

  • A test with 2 OpenMP threads, to confirm thread safety. case-a01 will be renamed to case_8_2threads
  • An event-based mode test, this is case-e01 to be renamed case_9_event_mode, since the above cases primarily use history-based simulations. I'll consider adding another event-based test with 2 threads, to fully ensure thread safety in the event-based solver too.

Let me know if this makes sense or if you have any other suggestions! Upon your confirmation on the regression test strategy, I will make the changes and add them to this PR asap.

saliba170499 avatar Jun 23 '25 14:06 saliba170499

@paulromano , thanks for the updates! I reviewed the changes and everything looks good on my side. 👍

saliba170499 avatar Nov 03 '25 19:11 saliba170499

Very cool PR! I'm a bit out of the loop, what kinds of applications would this collision tracking data be used for?

yardasol avatar Nov 14 '25 14:11 yardasol

Very cool PR! I'm a bit out of the loop, what kinds of applications would this collision tracking data be used for?

Thanks! it is utilized mainly for detector response and neutron noise simulation. check out these recent conference publication we wrote on the applications on this feature: https://doi.org/10.1051/epjconf/202533804001 and https://doi.org/10.13182/xyz-47184

saliba170499 avatar Nov 14 '25 15:11 saliba170499