Skip to content

Fail loudly when a score has no implementation for the estimator or energy mode - #4109

Open
GuySten wants to merge 1 commit into
openmc-dev:developfrom
GuySten:loud-default-scores
Open

Fail loudly when a score has no implementation for the estimator or energy mode#4109
GuySten wants to merge 1 commit into
openmc-dev:developfrom
GuySten:loud-default-scores

Conversation

@GuySten

@GuySten GuySten commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

Tally scores are dispatched through three switch statements — score_general_ce_nonanalog,
score_general_ce_analog and score_general_mg. Whether a score is actually implemented is
determined solely by whether it has a case label in the relevant one. Nothing connects those
switches to the validation in Tally::set_scores, so when a score is missing a case, the
default block silently skips it and the tally reports zero.

Enumerating the case labels shows four gaps today:

score tracklength/collision analog multigroup
fission-q-prompt yes yes missing
fission-q-recoverable yes yes missing
ifp-* (3 scores) yes missing missing
migration-area (#3810) yes missing yes

These are reachable. set_scores forces IFP scores to a collision estimator, but the
<estimator> element is read afterwards and its analog branch overwrites the estimator
unconditionally, so estimator='analog' on an IFP tally yields silent zeros. The multigroup
whitelist only rejects sc > 0 (MT reaction rates), so the negative pseudo-scores pass through
it untouched.

This PR does not add the missing implementations. It makes the absence loud:

  • The default block in each of the three dispatch functions now raises an error when
    score_bin < 0. Every pseudo-score is negative and every MT reaction rate is positive, so
    the sign cleanly separates "should have had a case" from the intended MT fallthrough.
  • fission-q-prompt / fission-q-recoverable are additionally rejected up front in
    multigroup mode, where kappa-fission is the available equivalent, so users get an early
    error with a useful suggestion rather than a late one.
    This follows the pattern validate_random_ray_inputs and FlatSourceDomain already use — an
    up-front whitelist plus a loud default in the scoring path.

Checked that the guard cannot fire on a valid configuration:

  • The two goto default_case jumps come from N_2N/N_3N/N_4N/N_GAMMA/N_P/N_A, all
    positive MTs.
  • SCORE_CURRENT and SCORE_PULSE_HEIGHT have no case in any of the three functions, but both
    set type_ away from VOLUME in set_scores, and setup_active_tallies routes by type_.
    score_surface_tally and score_pulse_height_tally handle them inline without delegating.
  • Random ray never reaches these functions; it has its own dispatch and its own whitelist.

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)

@GuySten
GuySten requested a review from paulromano September 2, 2026 21:19
@GuySten
GuySten marked this pull request as ready for review September 2, 2026 21:19
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.

1 participant