Skip to content

adding trigger to mgxs convert#3925

Draft
shimwell wants to merge 3 commits into
openmc-dev:developfrom
shimwell:trigger-mgxs-generation
Draft

adding trigger to mgxs convert#3925
shimwell wants to merge 3 commits into
openmc-dev:developfrom
shimwell:trigger-mgxs-generation

Conversation

@shimwell

Copy link
Copy Markdown
Member

Description

At Physor 2026 an chatting with @jtramm and @pshriwise about making generation of multigroup use a trigger to allow it to end earlier if the reaction rates all meet the trigger relative error value. At the time it sounded like a good idea but in practice it appears to have enlarged the api interface to users which I was trying to avoid. I wanted to make this a draft PR just to show @jtramm and @pshriwise that I tried.

Fixes # (issue)

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) 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)

@shimwell shimwell requested a review from pshriwise as a code owner April 22, 2026 13:10
@shimwell shimwell marked this pull request as draft April 22, 2026 13:10
@jon-proximafusion jon-proximafusion force-pushed the trigger-mgxs-generation branch from 6568bb7 to fba86e3 Compare June 23, 2026 12:03
@jon-proximafusion

jon-proximafusion commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

This PR has been updated and is now up to date with develop.

The auto-trigger settings are hardcoded in the source and used only when the user does not set nparticles. No new arguments were added.

So you keep exactly the control you have today — pass an integer for a fixed particle count — but you can now also leave nparticles unset and get an MGXS library whose cross-section tallies are converged to a 1% relative error.

before PR

model.convert_to_multigroup(nparticles=2000)  # guess a good number of particles, perhaps too few or perhaps excessive

after PR

model.convert_to_multigroup(nparticles=2000)  # you can still guess a good number of particles
model.convert_to_multigroup()  # let the trigger stop the simulation when converged

convert_to_multigroup now defaults nparticles=None ("auto"). In auto mode the
continuous-energy generation run is no longer given a fixed, guessed particle
count; a relative-error trigger (ignore_zeros=True) is attached to the
cross-section tallies via Library.tally_trigger, so the run extends batches
until the group constants are statistically converged, bounded by an internal
max_batches cap. Passing an explicit integer keeps the previous fixed behaviour.

The convergence target, seed particles-per-batch and max-batches cap are
internal constants rather than new user-facing arguments, so the public API
gains no knobs.
@jon-proximafusion jon-proximafusion force-pushed the trigger-mgxs-generation branch from fba86e3 to 9f1dea8 Compare June 23, 2026 12:15
@jon-proximafusion

Copy link
Copy Markdown
Contributor

@jtramm and @pshriwise I think this PR is better but still not very good as initially we had the issue of too many user args, now I think we have the issue of hard coded triggers, batches, particles

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