Add Tally Support ... Finally by MicahGale · Pull Request #1005 · idaholab/MontePy

MicahGale · 2026-08-05T12:33:53Z

Pull Request Checklist for MontePy

Description

Adds a full object model for MCNP tally (F card) and tally-multiplier (FM card) inputs, so tallies can be read, inspected, built from scratch, cloned, and edited like any other MontePy object instead of being treated as opaque text.

Highlights:

Tally base class with a concrete subclass per tally type (SurfaceCurrentTally, SurfaceFluxTally, CellFluxTally, DetectorTally, EnergyDepositionTally, FissionEnergyDepositionTally, EnergyDetectorPulseTally), selected automatically from the F-card's type digit via Tally.from_input.
Full support for tally scoring-group syntax: flat cell/surface lists, union (parenthesized) bins, universe-path (<) chains, and lattice index ([i j k]) specifications, modeled as TallyGroup/FlatGroup/PathGroup/LatticeIndex.
TallyMultiplier (FM card) object model, including the reaction-number DSL (Reaction, ReactionExpression) with +/*/- operator overloading matching MCNP's FM reaction-list grammar, attenuator sets, and special multipliers.
New builder API for constructing tallies/groups from scratch: SurfaceTally.add_surface/add_group/add_path_group, CellTally.add_cell/add_group/add_path_group, PathGroup.inside for chaining.
Cell.tallies and MCNP_Problem.tallies (a new Tallies collection) for discovering/iterating tallies.
Tally, TallyMultiplier, Reaction, all Tally subclasses, Tallies, and MultiplierSet are exported at the top level (montepy.Tally, etc.).
New "Tallies" guide page under Getting Started, and a "Tallies" section in the API docs, mirroring the Material section.

Fixes #11

TODO

Note on LLM Use

This was an experiment with "vibe coding". Besides what I wrote a few years ago I tried to do everything through Claude code. Though I feel like I was more involved than true "vibe coding".

Overall, while it did end decently, I still don't think it was worth it. I felt too detached from the design process, and not engaged enough that I felt confident with it. I will review all of the code first though.

I had to do a lot of micromanaging to get the user guide to actually be well written and well styled, and in my voice.

General Checklist

I have performed a self-review of my own code.
The code follows the standards outlined in the development documentation.
I have formatted my code with black version 25 or 26.
I have added tests that prove my fix is effective or that my feature works (if applicable).

LLM Disclosure

Are you?
- A human user
- A large language model (LLM), including ones acting on behalf of a human
Were any large language models (LLM or "AI") used in to generate any of this code?

Yes
- Model(s) used: Claude (Anthropic), via Claude Code
No

Documentation Checklist

I have documented all added classes and methods.
I have added type hints to all functions as needed.
I have marked all changes with the .. versionchanged:: or .. versionadded:: directives.

Infrastructure Changes

For infrastructure updates, I have updated the developer's guide.

Significant features or Behavior changes

For significant new features, I have added a section to the getting started guide.

First-Time Contributor Checklist

If this is your first contribution, add yourself to pyproject.toml if you wish to do so.

Additional Notes for Reviewers

Ensure that:

This PR fully addresses and resolves the referenced issue(s).
The submitted code is consistent with the merge checklist outlined here.
The PR covers all relevant aspects according to the development guidelines.
100% coverage of the patch is achieved, or justification for a variance is given.

📚 Documentation preview 📚: https://montepy--1005.org.readthedocs.build/en/1005/

DataLexer's FILE_PATH pattern consumes `[` and `]`, preventing them from being matched as literals. TallyLexer narrows FILE_PATH to exclude those characters and adds them as literals so `[0 0 0]` tokenizes correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds grammar rules for: - Path separators: (1<2<5) - Lattice element indices: (1[0 0 0]<2) - Lattice ranges: (1<2[0:1 0:1 0:0]<5) - Comma-separated lattice sets: (1<2[0 0 0, 0 1 0]<5) - Nested sub-paths: (1<(2[0 0 0] 2[0 1 0])<5) - Universe references: (u=1<2<5), ((u=1)<2<5) tally_numbers uses fresh rules (no inherited number_sequence) so the LALR(1) conflict between lparen_phrase and the parenthetical number_sequence production is eliminated — those states are unreachable from the `tally` start symbol. All new rule names are distinct from inherited DataParser rules to avoid MetaBuilder merge side-effects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Six bugs prevented TallyParser (and TallySegmentParser) from ever being used; all tally inputs silently fell back to DataParser: 1. _load_correct_parser stored a parser instance instead of the class, so _parse_input's self._parser() call double-instantiated it. 2. DataInput.__init__ did not propagate _prefix when full_parse() called it without a prefix argument, so _load_correct_parser was skipped. 3. parse_data did not pass prefix= to the final DataInput() call. 4. Input.tokenize() had no lexer_class parameter, so parser-specific lexers (e.g. TallyLexer) could not be selected. 5. _parse_input did not forward parser._lexer_class to tokenize(). 6. DataInput._KEYS_TO_PRESERVE did not include _prefix, so the prefix was not guaranteed to survive the JIT-to-full-parse transition. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

TestTallyPathSyntax directly invokes TallyParser + TallyLexer against all 11 complex tally forms from test_tally.imcnp. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add a full Tally class hierarchy (SurfaceCurrentTally, SurfaceFluxTally, CellFluxTally, DetectorTally, EnergyDepositionTally, FissionEnergyDepositionTally, EnergyDetectorPulseTally) with JIT parsing, subclass dispatch via Tally.from_input(), and correct handling of path/ lattice syntax in tally groups. Key changes: - tally.py: full rewrite with TallyGroup, FlatGroup, PathGroup, LatticeIndex helpers; lattice phrase and universe phrase parsing; link_to_problem uses problem._surfaces/_cells to avoid __relink_objs recursion - tally_parser.py: fix tally_group_body to preserve lattice_phrase and universe_phrase ListNodes intact (only flatten ShortcutNode) - tallies.py: inherit from NumberedDataObjectCollection (not NumberedObjectCollection) so insert_in_data kwarg is supported - data_parser.py: register Tally and dispatch via Tally.from_input() - mcnp_problem.py: add tallies property and load tallies during parse - cell.py: add tallies generator property (scanning pattern) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

tjlaboss

Had some time during updates so gave this draft PR a first pass.

I've looked over everything except the largest and most complex changes: tally.py, tally_multiplier.py, tally_type.py, and tally_parser.py. Those will receive a more thorough review after this PR is out of draft status.

FMESH and TMESH tallies are not part of this PR, unless I missed something. Those are their own respective cans of worms. I concur with attacking those later.

The documentation, testing, and boilerplate all look sound to me. I've got some inputs with...creative...tally specifications which I would like to test this one. A few more unit tests could be inspired by those.

tjlaboss · 2026-08-28T21:34:01Z

+class Score(Enum):
+    """The physical quantity a :class:`~montepy.Tally` scores.
+
+    A shallow analog of OpenMC's tally scores: for MontePy this is just the
+    quantity implied by the tally type digit (e.g. F4 always scores
+    :class:`Score.FLUX`). If an FM tally-multiplier card is linked to the
+    tally, :attr:`~montepy.Tally.scores` returns a list of
+    :class:`~montepy.data_inputs.tally_multiplier.MultiplierScore` instead of
+    this enum -- see :attr:`~montepy.Tally.multiplier`.
+
+    .. versionadded:: 1.6.0b2
+    """
+
+    CURRENT = 1
+    FLUX = 2
+    ENERGY_DEPOSITION = 6
+    FISSION_ENERGY_DEPOSITION = 7
+    PULSE_HEIGHT = 8


Flux (fluence) over surface, cell, and various detector types should have nonambiguous names.

Values for +Fn tally types may need to be distinguished. An easy but potentially misleading way could be to use negative numbers for the +, e.g. COLLISION_HEATING = -6

I'm not sure I like this method; it feels MCNP hacky. Maybe we make the IDs tuples of (TYPE, modified).

We could do that, but if it's truly a binary, then doing it with a sign flip is cleaner.

The two other ways that I see are including an additional digit COLLISION_HEATING = 16 (also potentially confusing), or using a StrEnum with e.g. COLLISION_HEATING = "+6"

My hesitation is that the sign would be opposite of the intuition. We probably would want positive to be the unmodified, which then leads to + = -.

Going with StrEnum would be more sensible for the four F5 variants.

First design question: do we continue the general mapping of 1 mnemonic to 1 class? I.e., would FIP, FIR, FIC be their own classes? I think yes; but let's subclass Tally.

Overall design wise I like the idea of this enum being for all tallies; not just F type. I'm thinking maybe we do: (mnemonic, modulo, modifier). This way it can be easily extended to TMESH and FMESH.

Next design question: are ENERGY_DEPOSITION and COLLISION_HEATING different Tally subclasses?

do we continue the general mapping of 1 mnemonic to 1 class?

Yes.

are ENERGY_DEPOSITION and COLLISION_HEATING different Tally subclasses?

Also yes.

tjlaboss · 2026-08-28T21:39:05Z

    @_(
-        '"(" number_sequence ")"',
-        '"(" padding number_sequence ")"',
+        "number_phrase",
+        "null_phrase",
+        "shortcut_phrase",
+        "path_sep",
+        "lattice_phrase",
+        "universe_phrase",
+        "tally_group",
+        "reaction_operator",
    )


TODO come back and review parsing carefully.

MicahGale · 2026-08-31T19:11:02Z

I've looked over everything except the largest and most complex changes: tally.py, tally_multiplier.py, tally_type.py, and tally_parser.py. Those will receive a more thorough review after this PR is out of draft status.

This is what is holding me up as well. I started reviewing it and realized it was missing a lot of features. It might be a bit before I have the time to marshal this to be ready for review.

…-deposition tally support. PR review feedback: bump stale 2024-only copyright headers, and give MCNP's +F6 (collision heating) and +F8 (charge deposition) tally variants their own Tally subclasses instead of leaving them unrepresentable. TallyType's value is now a (mnemonic, modulo, modifier) tuple rather than a bare digit, so +F6/+F8 can be distinct members that still share their digit with the unmodified card -- keeping TallyType general enough to cover non-F-card tallies (e.g. FMESH) later instead of narrowing it to "F-card digit only". Fixed JitDataParser to stop discarding a leading +/* modifier token instead of capturing it (a real, independent bug also affecting Transform/Fill's *TR/*FILL, which self-corrected only once a full parse happened to be triggered). Tally._update_values now syncs the classifier's modifier on every write, so clone_as correctly adds/drops the + in both directions. FIP/FIR/FIC and FMESH/TMESH remain explicitly out of scope, matching the existing reviewer consensus to defer them as their own future work.

…cstring. The :attr: reference pointed at the internal module path (montepy.data_inputs.tally.Tally.multiplier), but autosummary documents the class under its top-level alias (montepy.Tally), so the reference never resolved. Every other reference to this property in the codebase already uses the correct montepy.Tally.multiplier form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

MicahGale

Some preliminary review comments. I have not completely reviewed, and I figure my Tally feedback will lead to many more changes.

MicahGale · 2026-09-28T04:25:36Z

+        for surface in list(self._surfaces):
+            if surface not in self:
+                self._surfaces.remove(surface)


This seems like O(N^2) risk.

Confirmed, and worse than O(N²) in practice — remove_group's cleanup loop
called x not in self once per still-linked cell/surface, each of which
rescanned every remaining group; combined with each remove_group call
happening once per removed item, bulk removal was closer to O(N³). Added
Tally._referenced_objects_and_numbers(), computing the still-referenced
set once per call (O(groups)) instead of once per candidate item — N=800
singleton-group removals went from ~10s to well under a second. The
remaining O(N²) comes entirely from NumberedObjectCollection.remove()'s
own O(N) linear list removal, a separate, pre-existing, codebase-wide issue
affecting every collection, not just tallies — flagging as a known
follow-up rather than fixing it here.

Couldn't you just check self._surfaces instead of doing a tree walk for not in self?

MicahGale · 2026-09-28T04:27:43Z

+
+    @args_checked
+    @needs_full_cst
+    def remove_cell(self, cell: montepy.Cell) -> None:


Same user ambiguity.

Same fix as #45 — CellTally.add_path_group now passes the real cell
objects directly into FlatGroup(...) instead of numbers.

MicahGale · 2026-09-28T04:28:45Z

+                self._link_group_cells(group._levels[0], problem)
+
+
+class DetectorTally(Tally):


TODO: look into docs for DetectorTally for if we are missing any properties.

Noted as a follow-up documentation task; didn't investigate in this pass —
wanted to flag it rather than let it silently drop.

Review feedback (PR #1005): data_inputs/__init__.py imported Material and ThermalScatteringLaw twice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): explain why Tally alone needs Tally.from_input's factory dispatch instead of direct construction -- its prefix maps to multiple concrete subclasses (by type digit and +/modifier), unlike every other DataClass here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): tally.py's import block wasn't alphabetized by module path; tally_multiplier.py had the same issue, fixed for consistency while touching this code again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): explain why _TallyKey is a NamedTuple (hashability, needed for Enum's internal value lookup), document its previously-undocumented mnemonic attribute via a proper Attributes section, explain why _TallyKey.__repr__ is overridden, and document why Score is a deliberate, coarser many-to-one grouping over TallyType rather than a redundant duplicate (SURFACE_FLUX/CELL_FLUX/DETECTOR all collapse to Score.FLUX). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): LatticeIndex is an immutable value object (no setters), so its dimensions should be a tuple rather than a list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): .particles already returns a set, so the frozenset() wrapping was redundant -- no ephemeral-comparison hazard to guard against. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): _parse_tally_body and _number_validator both computed num % _TALLY_TYPE_MODULUS inline. Factored into a shared _digit_of() helper; the two call sites keep distinct semantics (one checks "is this digit valid for any tally type", the other "does this match this subclass's specific type"), noted via comments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): from_input's fallback caught bare Exception, which also silently swallowed a real MalformedInputError from _dispatch_class (e.g. an invalid digit/modifier combination) and retried via a full parse instead of letting it propagate directly. Narrowed to the specific exceptions the light JIT parser can actually raise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): a set has no guaranteed order, risking a non-deterministic FlatGroup number order across runs. add_group now accepts list | tuple instead of list | set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): code-format every bare F1-F8 tally mnemonic in prose; use `|` instead of comma-separated numpydoc type lists to match the real union type hints; replace bare int with Integral/ty.Integral/ty.PositiveInt in type hints and docstring prose (also fixes _has_classifier's return type mismatch against its base declaration); move clone_as's Note section before Parameters to match this codebase's house style (see Cells' docstring); fix "non-negative" wording to "positive" (the code already uses a positive placeholder); point clone_as's demo at the top-level montepy.EnergyDepositionTally instead of the internal montepy.data_inputs.tally module path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…flag. _update_values unconditionally wrote "T" on every format_for_mcnp_input call whenever include_total was true, silently normalizing a parsed lowercase "t" to uppercase even when include_total was never touched -- exactly the class of round-trip regression this branch has otherwise been eliminating. Track an as-parsed snapshot (mirroring TallyMultiplier._parsed_bins) and only rewrite the end node when the value actually changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…lem. All three overrides accepted a deepcopy kwarg but called super().link_to_problem(problem) without it, silently dropping the flag before it ever reached MCNP_Object.link_to_problem. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ad of hardcoding "F". Review feedback (PR #1005): _dispatch_class always built its TallyType lookup key with a hardcoded "F" mnemonic, even though TallyType's value shape already supports other mnemonics (e.g. a future FMESH). from_input now reads the actual parsed classifier prefix and passes it through in both the light-parse and full-parse fallback paths. This doesn't add FMESH support, but removes the hardcoded assumption blocking it later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

… a renumbered shortcut group. FlatGroup._update_node used to silently skip patching any group parsed with an MCNP shortcut (e.g. "3i"), since only the ShortcutNode itself knows how to recompress -- a cell/surface renumbered inside such a group would update cells_or_surfaces/_current_numbers() correctly but mcnp_str() would keep emitting the stale original text with no error. Now raises IllegalState instead. Making the group actually live-patchable is a larger, separate change (would need the shortcut handling moved into the grammar); this only makes the existing, accepted limitation loud instead of silent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…r call. remove_group's cleanup loop called "x not in self" once per still-linked cell/surface, each of which re-scanned every remaining group via Tally.__contains__ -- O(N*M) per call, confirmed empirically super-quadratic overall when groups are added one-at-a-time (e.g. many add_cell calls): N=800 took ~10s. Added Tally._referenced_objects_and_numbers(), computing the still-referenced set once per remove_group call (O(groups)) instead of once per candidate item. N=800 now takes well under a second. The remaining O(N^2) from NumberedObjectCollection.remove()'s own O(N) linear list removal is a separate, pre-existing, wider issue affecting every collection, not just tallies -- left for a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): "How does this perform a full tree walk?" -- it deliberately doesn't; only the innermost (scored) level counts, since an outer level is a containment constraint ("...inside cell 5"), not itself scored. Documented explicitly and locked in with a test against the F194:n (1 < 2 < 5) fixture, rather than leaving it looking like an oversight. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@AbstractMethod

Review feedback (PR #1005): both docstrings called themselves "Abstract base for..."/"Abstract analog of..." but neither inherited ABC, unlike every other "abstract base" class in the codebase (MCNP_Object, Numbered_MCNP_Object, NumberedObjectCollection). TallyGroup.__contains__ is now a real @AbstractMethod instead of a manual raise NotImplementedError. Filter gets no abstractmethod (no shared contract exists today beyond the docstring's intent), so this is a documentation-only marker for it -- ABC alone doesn't block instantiation without at least one abstract method. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): Tallies.step defaulted to NumberedObjectCollection's base of 1, so _next_number_for_type compensated by doing candidate += step * 10 to preserve the trailing type digit -- an internal doubling that silently broke if a caller ever set step to anything other than the old default (e.g. step=20 actually spaced candidates by 200, not 20). Tallies.step now defaults to 10 and rejects non-multiples of 10 via a validating setter override; _next_number_for_type just does candidate += step, making step's docstring-promised meaning literally true. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Review feedback (PR #1005): "The point is that clone is like for like: F4 in; F4 out. clone_as should be the only route to change F4 to F6. I don't see why multipliers should be dropped then." clone() keeps the same tally type, so a linked FM's scoring relationship is still meaningful for the clone -- uses TallyMultiplier.clone(tally=...) (the bespoke API built earlier this session for exactly this) to clone and link a fresh FM at the new tally's number. clone_as still drops the multiplier unconditionally, since it's a genuine type change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…MCNP_Object. Review feedback (PR #1005): "The whole point of a lot of my work was to remove shallow inits like this. You [should] change the inherit order to shift MRO priority." Tally(DataInputAbstract, Numbered_MCNP_Object) made super().__init__() land on DataInputAbstract.__init__, whose 2nd positional param is fast_parse, not number -- so __init__ called Numbered_MCNP_Object.__init__ directly instead. Reordered to Tally(Numbered_MCNP_Object, DataInputAbstract); Numbered_MCNP_Object's own super().__init__() call still correctly falls through to DataInputAbstract next. Full suite verifies no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ed_MCNP_Object. Same fix as Tally (3ee0e84): Material(DataInputAbstract, Numbered_MCNP_Object) made super().__init__() land on DataInputAbstract.__init__ instead of Numbered_MCNP_Object.__init__, forcing a direct Numbered_MCNP_Object.__init__(self, ...) call to work around it. Reordered to Material(Numbered_MCNP_Object, DataInputAbstract). Full suite verifies no behavior change (the 25 pre-existing doctest failures on this branch are unrelated -- confirmed via git stash they're present with or without this change; they're caused by a missing foo.imcnp fixture file, not this refactor). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

… Numbered_MCNP_Object. Same fix as Tally (3ee0e84) and Material (506bf28): TallyMultiplier(DataInputAbstract, Numbered_MCNP_Object) made super().__init__() land on DataInputAbstract.__init__ instead of Numbered_MCNP_Object.__init__, forcing a direct Numbered_MCNP_Object.__init__(self, ...) call to work around it. Reordered to TallyMultiplier(Numbered_MCNP_Object, DataInputAbstract). Full suite verifies no behavior change. Transform intentionally left untouched -- it uses a different, already-correct init idiom, not the shallow-init pattern shared by these three. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ch before forcing a full parse. Found while discussing how Tally's JIT parsing interacts with reverse lookups like Cell.tallies: it (and 5 similar generators -- Cell.cells_complementing_this, Universe.cells/filled_cells, Material.cells, Surface.cells) scanned every candidate object in the problem and forced a full parse of each one (via a membership/equality check gated behind @needs_full_ast) just to answer "does this one specific object reference me" -- catastrophic for a problem with many still-JIT objects, since one query would fully parse the entire collection. Applies the pre-filter pattern already established elsewhere in the codebase (CellModifierInput._check_redundant_definitions, UniverseInput.push_to_cells's grab_cells_from_jit_parse): for a still-JIT candidate, do a cheap MCNP_Object.search() against its raw input text first (no parse needed), and only pay for a full parse if the search plausibly matches. Uses a word-boundary regex (stricter than the existing precedent's plain substring search) to avoid false-positive full parses from number collisions (e.g. searching for cell 1 no longer matches "1005"). Searches both the target's current and as-originally-parsed number, since a candidate written before a renumber may still reference the old number. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…o tally_group.py. Review feedback (PR #1005): FlatGroup/PathGroup stored raw cell/surface numbers plus a hand-synced _cells_or_surfaces side list, unlike the established HalfSpace/UnitHalfSpace pattern (store whatever's given, resolve via update_pointers). This redesigns the whole scoring-group object model: - New montepy/data_inputs/tally_group.py (mirrors half_space.py being its own file): LatticeIndex, TallyGroup, Filter, ParticleFilter, SpatialFilter, FlatGroup, PathGroup, and the group-parsing helpers moved out of tally.py, which now imports (and re-exports) them. - LatticeIndex folds the old free functions _build_lattice_node/ _parse_lattice_phrase into a .node property and .parse_input_node() classmethod, mirroring HalfSpace.parse_input_node. - FlatGroup.__init__ now accepts raw numbers or live Cell/Surface objects directly (list[Integral | Cell | Surface]); add_cell/ add_group/add_path_group/PathGroup.inside pass objects straight in instead of computing numbers by hand and patching _cells_or_surfaces afterward. A frozen _old_numbers snapshot is kept separately (numbers as originally read/added), since it's still needed for the shortcut-staleness check and the old_numbers property's "as parsed" contract -- confirmed empirically that always resolving live would silently defeat both. - New FlatGroup.update_pointers()/PathGroup.update_pointers() replace the duplicated _link_group_cells/_link_group_surfaces logic in SurfaceTally/CellTally with one shared implementation, and raise BrokenObjectLinkError on a genuinely missing number (matching UnitHalfSpace.update_pointers) instead of silently skipping it via a bare "except KeyError: continue". PathGroup.update_pointers only resolves the innermost (scored) level -- per the MCNP manual's Si/Ci distinction (only the leftmost tally cell is scored; outer levels are containment cells filled with a universe, never themselves tallied), an outer level's numbers aren't worth live renumber-tracking. - New FlatGroup.cells/.surfaces properties return the resolved objects typed to Cell/Surface respectively, or None if the group isn't (yet, or ever) that kind; cells_or_surfaces is kept as the existing dual-purpose accessor. Fallout from strict linking, handled in this same commit since the redesign isn't meaningfully separable from it: - tests/inputs/test_tally.imcnp: added cells 4/7/8/9, which f44/f54's groups reference but which didn't exist in the fixture -- previously silently tolerated, now they must resolve. - 5 tests in test_tally_multiplier.py constructed a CellTally inside a cell-less MCNP_Problem purely to test multiplier registration/ renumbering/cloning; added a problem_with_cells fixture (backed by a small real .imcnp with valid geometry, needed for the one test that does a full problem export/re-parse) so they resolve instead of raising. - Updated the one existing test asserting SurfaceTally's old recurse-all-path-levels behavior (now innermost-only, matching CellTally and the manual) and the one asserting silent-skip-on- missing-surface (now BrokenObjectLinkError). Full suite (1699 passing) and doc build (doctest + html) both clean; the only remaining html warnings are pre-existing, unrelated montepy.types cross-reference issues confirmed via git stash to predate this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

MicahGale · 2026-09-29T15:14:04Z

Some more high-level design comments.

* is another tally modifier. From section 5.9.1 of the 6.3.1 manual:

Tally types 1, 2, 4, and 5 are normally weight tallies; however, if the F card is flagged with an asterisk (for example, * F1 :n), energy times weight will be tallied. The asterisk flagging also can be used on tally types 6 and 7 to change the units from MeV/g to jerks/g. No asterisk can be used in combination with the + on the + F8 or + F8 tallies. The asterisk on a tally type 8 converts from a pulse-height tally to an energy deposition tally. All of the units are shown in the Table 5.18.

F5 detector tallies have their own simpler syntax. Given this I think we will punt F5. They should just be simply parsed like unimplemented data inputs, and an issue opened to add support for them later.

MicahGale and others added 30 commits January 13, 2024 22:08

Enforced unique surface types.

c72e0aa

Added tallies object.

a502033

Listed tally types allowed.

8fbe84e

Started parsing tally objects.

7d26ebd

Flattened tally specification data structure for parsing.

2bb0cce

keep parens as valueNode not PaddingNode.

8a38f16

Started group parsing mechanism

94d0217

Implemented append for tally groups.

d98e63e

Made tally appendable.

8c652ca

Added particle type to test to be accurate.

42cca61

Merge branch 'develop' into tallies

751dac4

Moved to dev version.

38e97c4

Merge branch 'develop' into tallies

0c15319

Merge branch 'develop' into tallies

ba7db74

Merge branch 'develop' into tallies

c94c7f4

fixed circular import error.

c03132a

Merge branch 'develop' into tallies

bcaedef

Merge branch 'develop' into tallies

fed2127

Merge branch 'develop' into tallies

4b95c78

Merge branch 'develop' into tallies

0c761d3

Merge branch 'develop' into tallies

fd223f5

Claude: added tally test input file.

5efa86e

Claude: tried to cover all syntax edge cases.

3761507

Merge branch 'jit_parse' into tallies

8f72685

Add tests for TallyParser path and lattice syntax

6f3a1f1

TestTallyPathSyntax directly invokes TallyParser + TallyLexer against all 11 complex tally forms from test_tally.imcnp. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge branch 'beta-test' into tallies

b47ed45

tjlaboss reviewed Aug 28, 2026

View reviewed changes

MicahGale and others added 2 commits September 11, 2026 21:25

MicahGale commented Sep 28, 2026

View reviewed changes

MicahGale and others added 24 commits September 28, 2026 07:13

Claude: remove duplicate Material/ThermalScatteringLaw imports.

a514425

Review feedback (PR #1005): data_inputs/__init__.py imported Material and ThermalScatteringLaw twice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Claude: store LatticeIndex.dimensions as an immutable tuple.

894344b

Review feedback (PR #1005): LatticeIndex is an immutable value object (no setters), so its dimensions should be a tuple rather than a list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Claude: simplify ParticleFilter.__eq__.

d9cd733

Review feedback (PR #1005): .particles already returns a set, so the frozenset() wrapping was redundant -- no ephemeral-comparison hazard to guard against. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Claude: drop set from add_group's accepted types.

22855c6

Review feedback (PR #1005): a set has no guaranteed order, risking a non-deterministic FlatGroup number order across runs. add_group now accepts list | tuple instead of list | set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

		self._link_group_cells(group._levels[0], problem)


		class DetectorTally(Tally):

Conversation

MicahGale commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist for MontePy

Description

TODO

Note on LLM Use

General Checklist

LLM Disclosure

Documentation Checklist

Infrastructure Changes

Significant features or Behavior changes

First-Time Contributor Checklist

Additional Notes for Reviewers

Uh oh!

tjlaboss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MicahGale Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MicahGale commented Aug 31, 2026

Uh oh!

MicahGale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MicahGale commented Sep 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MicahGale commented Aug 5, 2026 •

edited

Loading

MicahGale Sep 8, 2026 •

edited

Loading