Skip to content

fix: parse multiply shortcuts in universe data cards#975

Open
chenlichao wants to merge 1 commit into
idaholab:developfrom
chenlichao:fix/universe-m-shortcut
Open

fix: parse multiply shortcuts in universe data cards#975
chenlichao wants to merge 1 commit into
idaholab:developfrom
chenlichao:fix/universe-m-shortcut

Conversation

@chenlichao

@chenlichao chenlichao commented Jun 27, 2026

Copy link
Copy Markdown

Pull Request Checklist for MontePy

Description

Fixes #500.

This PR fixes parsing for MCNP universe data cards that use multiply shortcuts, for example:

U J 1 2M 5M J 3 7M 1M

Bug being fixed: MontePy did not correctly preserve expanded integer identifiers when a universe data card used shortcut tokens such as 2M, 5M, 7M, or 1M. Those values could fall back through the syntax tree as text-like values instead of the numeric values required by the universe parser.

Cause: NUM_MULTIPLY data-card tokens were not being converted through the same numeric-shortcut path used by other parsed numeric values. When ValueNode instances were later converted to integer identifiers, shortcut-expanded values were not consistently available.

Fix: The parser now treats NUM_MULTIPLY as a numeric shortcut token and preserves the expanded numeric shortcut result on the syntax node, so downstream integer conversion receives the correct value.

Syntax rule impact: This keeps MCNP multiply shortcut handling local to numeric data-card syntax: nM expands according to the existing shortcut semantics, and the expanded values are used when constructing universe identifiers. The regression test covers the concrete issue input and verifies the resulting universe sequence.


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

  1. Are you?

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

  • Yes
    • Model(s) used: GPT-5.5 (OpenAI Codex), via Hermes Agent
  • No

Documentation Checklist

  • I have documented all added classes and methods. Added changelog entry in doc/source/changelog.rst.
  • For infrastructure updates, I have updated the developer's guide.
  • 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.

Test Plan

  • .venv/bin/python -m pytest tests/test_universe.py -q — 40 passed
  • cd doc && make html SPHINXOPTS='-W --keep-going -E' — build succeeded

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


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

@tjlaboss

Copy link
Copy Markdown
Collaborator

Please use the pull request template.

AI contributions are allowed, but must be disclosed.

@MicahGale

MicahGale commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

@chenlichao if the PR template is not used this PR will be closed in one week.

@chenlichao chenlichao force-pushed the fix/universe-m-shortcut branch from afed6ea to 42db9b8 Compare July 4, 2026 05:10
@MicahGale

MicahGale commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

@chenlichao please expand your description to include things like:

  1. What the bug being fixed is.
  2. What was causing the bug
  3. How this bug was fixed.
  4. How exactly the new syntax rules fix this bug.

For fixing the test failures:

  1. changelog-test please update doc/source/changelog.rst accordingly.
  2. doc-build this should now be fixed in develop just merge or rebase that in.

@MicahGale MicahGale self-requested a review July 7, 2026 22:24
@MicahGale MicahGale self-assigned this Jul 7, 2026
@MicahGale MicahGale added bugs A deviation from expected behavior that does not reach the level of being reportable as an "Error". parsers are hard Examples of where MCNP syntax is complicated and should be simplified. labels Jul 7, 2026
@chenlichao chenlichao force-pushed the fix/universe-m-shortcut branch from 42db9b8 to 388c356 Compare July 9, 2026 00:27
@chenlichao

Copy link
Copy Markdown
Author

Thanks for the guidance. I rebased this onto current develop, added the changelog entry, and expanded the PR description to cover the bug, root cause, fix, syntax behavior, and LLM disclosure. I also verified tests/test_universe.py and the strict docs build locally.

@chenlichao chenlichao force-pushed the fix/universe-m-shortcut branch from 388c356 to 907a6d1 Compare July 11, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugs A deviation from expected behavior that does not reach the level of being reportable as an "Error". parsers are hard Examples of where MCNP syntax is complicated and should be simplified.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🎉500 Issues🎉 The 'xM' shortcut can fail to parse

3 participants