Skip to content

Extend ParticleProductionFilter to support multiple particle types#3780

Open
paulromano wants to merge 14 commits intoopenmc-dev:developfrom
paulromano:particle-prod-multiple
Open

Extend ParticleProductionFilter to support multiple particle types#3780
paulromano wants to merge 14 commits intoopenmc-dev:developfrom
paulromano:particle-prod-multiple

Conversation

@paulromano
Copy link
Contributor

Description

This PR extends ParticleProductionFilter to allow tallying production of multiple secondary particle types in a single filter, avoiding the need for multiple tallies when tracking different secondary particles. The constructor has been changed from ParticleProductionFilter(particle, bins) to ParticleProductionFilter(particles, energies=None), where particles accepts a single particle or list of particles, and energies is now optional (defaulting to None for no energy binning). The filter becomes two-dimensional with shape (n_particles, n_energy_bins) and bins are flattened as particle_index * n_energy_bins + energy_bin. Using the filter to get particle production rates looks like:

# Track photon and neutron production in 3 energy groups
production_filter = openmc.ParticleProductionFilter(
    particles=['photon', 'neutron'],
    energies=[0.0, 1e5, 1e6, 20e6]
)

tally = openmc.Tally()
tally.filters = [production_filter]
tally.scores = ['events']

One issue that needed to be fixed was that the "events" score was hard-coded to be a value of 1.0 per tally contribution regardless of filter weights. Because ParticleProductionFilter sets the filter weight to the weight of the secondary particle, that needs to be accounted for. I've updated the logic so that the unity value gets multiplied by the filter weight before accumulation.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • 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)

Copy link
Contributor

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

Nice!! New valuable capability fits pretty cleanly into the tally system.

A few minor inline comments from me and one larger thought:

On the C++ side the ParticleProductionFilter inherits from EnergyFilter, but we re-implement almost every method of the parent class. From what I can see, what is reused from the EnergyFilter class is the bins_ vector and use of the set_bins method, which sets the matches_transport_groups attribute that isn't used in the new class. In contrast to the EnergyOutFilter that re-implements only a couple of methods of the EnergyFilter, this inheritance feels more awkward and like it might warrant it's own set of source files. Not a deal breaker for me on merging this, just sharing observations and something to consider.

@paulromano
Copy link
Contributor Author

@pshriwise Your comments have been addressed. Let me know if you have any further requests!

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