Fail loudly when a score has no implementation for the estimator or energy mode - #4109
Open
GuySten wants to merge 1 commit into
Open
Fail loudly when a score has no implementation for the estimator or energy mode#4109GuySten wants to merge 1 commit into
GuySten wants to merge 1 commit into
Conversation
GuySten
marked this pull request as ready for review
September 2, 2026 21:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Tally scores are dispatched through three switch statements —
score_general_ce_nonanalog,score_general_ce_analogandscore_general_mg. Whether a score is actually implemented isdetermined solely by whether it has a
caselabel in the relevant one. Nothing connects thoseswitches to the validation in
Tally::set_scores, so when a score is missing a case, thedefaultblock silently skips it and the tally reports zero.Enumerating the case labels shows four gaps today:
fission-q-promptfission-q-recoverableifp-*(3 scores)migration-area(#3810)These are reachable.
set_scoresforces IFP scores to a collision estimator, but the<estimator>element is read afterwards and its analog branch overwrites the estimatorunconditionally, so
estimator='analog'on an IFP tally yields silent zeros. The multigroupwhitelist only rejects
sc > 0(MT reaction rates), so the negative pseudo-scores pass throughit untouched.
This PR does not add the missing implementations. It makes the absence loud:
defaultblock in each of the three dispatch functions now raises an error whenscore_bin < 0. Every pseudo-score is negative and every MT reaction rate is positive, sothe sign cleanly separates "should have had a case" from the intended MT fallthrough.
fission-q-prompt/fission-q-recoverableare additionally rejected up front inmultigroup mode, where
kappa-fissionis the available equivalent, so users get an earlyerror with a useful suggestion rather than a late one.
This follows the pattern
validate_random_ray_inputsandFlatSourceDomainalready use — anup-front whitelist plus a loud default in the scoring path.
Checked that the guard cannot fire on a valid configuration:
goto default_casejumps come fromN_2N/N_3N/N_4N/N_GAMMA/N_P/N_A, allpositive MTs.
SCORE_CURRENTandSCORE_PULSE_HEIGHThave no case in any of the three functions, but bothset
type_away fromVOLUMEinset_scores, andsetup_active_talliesroutes bytype_.score_surface_tallyandscore_pulse_height_tallyhandle them inline without delegating.Checklist
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)