Skip to content

support column format#196

Merged
dodu94 merged 2 commits intodevelopingfrom
support-column-format-for-cards
Mar 3, 2026
Merged

support column format#196
dodu94 merged 2 commits intodevelopingfrom
support-column-format-for-cards

Conversation

@dodu94
Copy link
Member

@dodu94 dodu94 commented Mar 3, 2026

Description

Support column format in MCNP. Something like:

wwe:p    1.e-6 1.e-5 1.e-4 .001 .01 .1    16.
    #     wwn1:p    wwn2:p    wwn3:p    wwn4:p    wwn5:p    wwn6:p    wwn7:p
          1  3.54E+00  2.35E+00  7.93E-01  5.00E-01  5.58E-01  1.29E+00  5.00E-01
          2  4.00E+00  0.00E+00  1.18E+00  4.66E-01  6.17E-01  1.75E+00  4.00E+00
          3  4.00E+00  0.00E+00  1.21E+00  5.83E-01  5.82E-01  1.98E+00  4.00E+00

Type of change

Please select what type of change this is.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
    • Non-breaking change which adds functionality
    • Breaking change fix or feature that would cause existing functionality to not work as expected

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • General testing
    • New and existing unit tests pass locally with my changes
    • Coverage is >80%

Summary by CodeRabbit

  • New Features

    • Support for column-formatted MCNP input lines; such blocks are now merged into their parent cards and processed correctly.
  • Tests

    • Added test coverage for column-formatted input handling to ensure reliable round-trip output.
  • Refactor

    • Updated the internal numerical integration used in ACE group-structure energy collapse (no change to user-facing behavior).

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

Walkthrough

Detects column-formatted MCNP lines beginning with # and merges them into the preceding card by appending those lines and re-parsing the previous card; also updates an ACE integration call and adds a unit test for the column-format behavior.

Changes

Cohort / File(s) Summary
MCNP Column-Format Parser Enhancement
src/f4enix/input/MCNPinput.py
Adds PAT_COLUMN_FORMAT = re.compile(r"\s*\#") and logic in _to_dict to append column-formatted lines (starting with #) to the previous card, then re-parse that card to refresh its state and continue processing.
Column-Format Feature Test
tests/MCNPinput_test.py
Adds test_column_format to verify that column-formatted lines are preserved through write/read and that a specific formatted comment line ( # wwn1:p) appears in output.
ACE integration function rename
src/f4enix/input/ace.py
Replaces numpy.trapz(y, x) with numpy.trapezoid(y, x) in group-structure energy collapse integration (functionality unchanged).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudge the hashes into line,
Cards re-parsed — they look divine,
A tiny hop, a gentle fix,
Comments nestled, neat and mixed. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'support column format' directly and concisely describes the main change—adding support for MCNP column-formatted lines in card parsing and output.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch support-column-format-for-cards

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dodu94 dodu94 requested a review from AlbertoBittes March 3, 2026 15:45
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/f4enix/input/MCNPinput.py`:
- Around line 605-613: The code assumes previous_key is always set when handling
column-format continuations (PAT_COLUMN_FORMAT.match(card.lines[0]) branch),
which can raise if a fragment appears before any real card; modify the
conditional to guard that previous_key is not None and exists in new_cards
(e.g., check "previous_key is not None and previous_key in new_cards") before
accessing new_cards[previous_key], and if the guard fails either create a new
entry or skip/raise a controlled parse error; apply the same guard to the
similar merge at the later occurrence (the block around the other
PAT_COLUMN_FORMAT usage at ~line 628) and ensure subsequent calls to get_input()
and get_values() only run on a valid card object.

In `@tests/MCNPinput_test.py`:
- Line 928: The test assigns an unused variable inp2 from
Input.from_input(outfile) causing a lint F841; either remove the assignment or
replace inp2 with a throwaway name (e.g., _) to indicate the parse is executed
for side-effects only. Locate the call to Input.from_input in the test (the inp2
variable assignment) and change the left-hand variable to _ or drop the
assignment entirely so the parsing is still performed but no unused-variable
warning is emitted.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cfeaa4 and 4a85eab.

📒 Files selected for processing (3)
  • src/f4enix/input/MCNPinput.py
  • tests/MCNPinput_test.py
  • tests/resources/input/column_format.i

@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/f4enix/input/MCNPinput.py 95.22% <100.00%> (+0.03%) ⬆️
src/f4enix/input/ace.py 85.00% <100.00%> (ø)

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/f4enix/input/ace.py`:
- Line 317: The call to np.trapezoid in the values.append(...) expression will
fail on NumPy <2.0.0; either add an explicit dependency numpy>=2.0.0 to
pyproject.toml, or add a local compatibility fallback before that call: resolve
a trapezoid function by checking getattr(np, "trapezoid", np.trapz) (or
try/except AttributeError) and then replace the direct np.trapezoid call with
the resolved function (e.g., trapezoid_fn(y, x) / (e - e_0)) so older NumPy
installations use np.trapz.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a85eab and 7887ab1.

📒 Files selected for processing (1)
  • src/f4enix/input/ace.py

@dodu94 dodu94 requested a review from AlvaroCubi March 3, 2026 16:11
@dodu94 dodu94 merged commit 1e101fa into developing Mar 3, 2026
8 checks passed
@dodu94 dodu94 deleted the support-column-format-for-cards branch March 3, 2026 16:13
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.

2 participants