Allow zero bins in tally triggers

[tjlaboss]

Resolve #2899

Description

Add the ability to ignore tally bins with zero scores when evaluating tally triggers.

• New allow_zero option for Trigger class and tallies.xml. (default: False)
• If there is a score without contributions, do not set the trigger threshold ratio to infinity if the trigger allows zero scores.
• Ensure that a full trigger batch interval is run before evaluating triggers (see discussion).
• Add a unit test. The O-16(n,p) reaction is tallied using an energy filter with two bins, one of which is entirely below the threshold energy. The low energy bin should always have a score of zero.
• Update "Tallies Specification" documentation.

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)

Questions and Discussion

OpenMC's tally trigger behavior

After OpenMC completes inactive batches, it runs a few active batches to begin gathering tally statistics. Then, tally triggers are evaluated.

This can present a problem with the new 'allow_zero' option. When triggers are evaluated this early, tally bins may not yet have any scores. Normally, this sets the error metric to infinity, but when allow_zero == True, the trigger can "fire" prematurely if none of the tally bins have any hits yet. This can happen for rare events, such as shielding problems and small-xs reactions.

Currently, this check is performed immediately after those initial active batches are performed, regardless of the user's tally trigger 'trigger_batch_interval' setting; let's call this "interval 0". In commit 29b0782, check_triggers() is changed to skip "interval 0", so that the check is only performed after each full interval. This way, users can ensure a minimum number of active batches and avoid false positives when zero-score bins are allowed.

This does change the default tally trigger behavior. Since the default 'trigger_batch_interval' is 1, this will add 1 additional batch before checking tally triggers even if they do not enable allow_zero on any tallies. I think this is the best balance between straightforwardly addressing the problem and being unobtrusive to existing tally trigger users, but let's discuss.

Alternatives

• One alternative would be to check if allow_zero is used on any tallies, and only skip the 0th interval if it is. The advantage is that this would then have no impact on existing workflows. The disadvantage is that the trigger checking behavior with and without the option would be inconsistent.
• Another alternative would be to continue checking at the 0th interval, document that this can cause problems with rare events, and make it the user's responsibility.

Testing

Is the single unit test sufficient, or is a regression test necessary as well?

Bool

How do we want to handle Booleans in the XML, without using eval (unsafe) or distutils (deprecated)? Here, I just check if a lowercase string is in ("true", "1"), or ("false", "0"), and otherwise crash. This is similar to how bools are handled in settings.py, but with the inconsistency that an unrecognized string (e.g., "rtue" or "yes") won't default to False.

[tjlaboss]

Triggers unsatisfied, max unc./thresh. is 1.0000000000000002 for (n,p) in tally 3

Wonder if this is floating point issue or a data library difference vs. the ENDF and JEFF libraries I tested on. Marking as draft...

...reproduced this using the NNDC data. Added more particles, and now the triggers are satisfied early as expected.

[paulromano]

@tjlaboss Thanks for implementing this! The code looks good to me. Two general comments:

• I think a name like ignore_zeros would more clearly express the intent of the option compared to allow_zero. Are you OK with renaming it as such?
• I see the point about skipping the first check, but that would eliminate the easy-to-remember notion that when using triggers, settings.batches is the minimum number of batches and settings.trigger_max_batches is the maximum. I personally think we should preserve the current behavior and users will just have to be cautious about the minimum number of batches chosen. The change to skip "interval 0" doesn't really eliminate the problem but rather defers it until "interval 1" where the same issue can still happen (all bins being zero).

[tjlaboss]

This makes sense to me. I'll rename the parameter to ignore_zeros, revert 29b0782, make a note in the documentation, and add more particles to the test case.

[paulromano]

Thanks @tjlaboss! Please ping me when the PR is ready for another look

[tjlaboss]

@paulromano, thanks for the review. The requested changes have been made and the CI tests have passed.

[paulromano]

Thanks @tjlaboss!