Adding OpenMC version of FNG-SS benchmark #475
Conversation
WalkthroughAdds OpenMC FNG-SS assets: documentation, an OpenMC benchmark YAML, test fixtures (geometry, materials, tallies, volumes, metadata), and pytest cases invoking raw data processing for OpenMC runs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
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. Comment |
jude-moo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/tallies.xml (1)
10-10: Consider cleaning up floating-point precision artifacts in energy values.The energy grid contains values with precision artifacts (e.g.,
8000000.999999999,16100000.000000002). While OpenMC likely handles these correctly, rounding to cleaner values would improve data quality and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/tallies.outis excluded by!**/*.out
📒 Files selected for processing (6)
tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/geometry.xml(1 hunks)tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/materials.xml(1 hunks)tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/metadata.json(1 hunks)tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/tallies.xml(1 hunks)tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/volumes.json(1 hunks)tests/post/test_raw_processor.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/dummy_structure/simulations/openmc-FENDL 3.2b/FNG-SS/FNG-SS_Al/metadata.json
- tests/dummy_structure/simulations/openmc-FENDL 3.2b/FNG-SS/FNG-SS_Al/volumes.json
🧰 Additional context used
🧬 Code graph analysis (1)
tests/post/test_raw_processor.py (2)
src/jade/config/raw_config.py (2)
ConfigRawProcessor(11-34)from_yaml(23-34)src/jade/post/raw_processor.py (2)
RawProcessor(15-118)process_raw_data(61-103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (windows-latest, 3.11, false)
- GitHub Check: test (ubuntu-latest, 3.11, false)
- GitHub Check: test (ubuntu-latest, 3.10, false)
- GitHub Check: test (ubuntu-latest, 3.12, false)
- GitHub Check: test (windows-latest, 3.10, false)
- GitHub Check: test (windows-latest, 3.12, false)
🔇 Additional comments (2)
tests/post/test_raw_processor.py (1)
356-369: LGTM!The test follows the established pattern for OpenMC tests, is properly gated with
pytest.mark.skipif(not OMC_AVAIL), and correctly loads the configuration and processes the simulation folders.tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/geometry.xml (1)
1-145: LGTM!The geometry definition is structurally sound and properly integrates with the tally configuration (cells 500–506 match the tally filter bins in tallies.xml). Vacuum boundaries are correctly defined on outer surfaces.
Note: Materials 12–15 from materials.xml are not referenced in this geometry, which supports the concern raised about those being potentially erroneous duplicates.
| <material id="12"> | ||
| <density units="g/cm3" value="2.7"/> | ||
| <nuclide ao="1.0" name="Fe56"/> | ||
| </material> | ||
| <material id="13"> | ||
| <density units="g/cm3" value="2.7"/> | ||
| <nuclide ao="1.0" name="Fe56"/> | ||
| </material> | ||
| <material id="14"> | ||
| <density units="g/cm3" value="2.7"/> | ||
| <nuclide ao="1.0" name="Fe56"/> | ||
| </material> | ||
| <material id="15"> | ||
| <density units="g/cm3" value="2.7"/> | ||
| <nuclide ao="1.0" name="Fe56"/> | ||
| </material> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check usage of materials 12-15 in geometry.xml
echo "=== Checking material references in geometry.xml ==="
for mat_id in 12 13 14 15; do
echo "Material $mat_id references:"
rg -n "material=\"$mat_id\"" "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/geometry.xml" || echo " (no matches found)"
doneRepository: JADE-V-V/JADE
Length of output: 289
🏁 Script executed:
#!/bin/bash
# Verify geometry.xml exists and check its structure
echo "=== Checking if geometry.xml exists ==="
ls -lh "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/geometry.xml" 2>/dev/null || echo "File not found"
echo -e "\n=== Checking how materials are referenced in geometry.xml ==="
head -100 "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/geometry.xml" | grep -i material | head -10
echo -e "\n=== Checking if materials 12-15 are referenced anywhere in the directory ==="
for mat_id in 12 13 14 15; do
count=$(rg -c "material[\"']?[=:]?\s*[\"']?$mat_id[\"']?" "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/" 2>/dev/null | wc -l)
echo "Material $mat_id appears in: $count files"
rg "material[\"']?[=:]?\s*[\"']?$mat_id[\"']?" "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/" -l 2>/dev/null
doneRepository: JADE-V-V/JADE
Length of output: 1164
🏁 Script executed:
#!/bin/bash
# Verify materials 12-15 in materials.xml
echo "=== Materials 12-15 in materials.xml (lines 122-137) ==="
sed -n '122,137p' "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/materials.xml"
echo -e "\n=== Total material definitions in file ==="
grep -c '<material id=' "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/materials.xml"
echo -e "\n=== All material IDs ==="
rg '<material id="(\d+)"' -o -r '$1' "tests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/FNG-SS/FNG-SS_Al/materials.xml" | sort -nRepository: JADE-V-V/JADE
Length of output: 670
Remove unused material definitions.
Materials 12–15 are defined identically (density 2.7 g/cm³, Fe56 composition) but are never referenced in geometry.xml or elsewhere in the simulation. These are orphaned definitions that should be deleted to keep the materials file clean.
🤖 Prompt for AI Agents
In tests/dummy_structure/simulations/_openmc_-_FENDL
3.2b_/FNG-SS/FNG-SS_Al/materials.xml around lines 122–137 there are four
identical, unused material blocks (id="12"–"15") that should be removed; delete
the entire <material>...</material> entries for ids 12–15 from the file and then
run a quick search/validation against geometry.xml and the rest of the
simulation to confirm no references remain.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
dodu94
left a comment
dodu94
left a comment
There was a problem hiding this comment.
Hello Jude, thanks for this and sorry for the delay. Tests have been finally fixed and we can proceed. Please could you just remove the FNG-SS entry from the run-cfg.yml? You did good at inserting it since it was missing but in my PR I added all the missing ones and I am afraid if we push both we would run in a conflict:
|
@dodu94 This should now be removed from the file. |
2 participants
This pull requests includes the required changes to add the FNG-SS benchmark for OpenMC into JADE. The changes include:
Summary by CodeRabbit
Documentation
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.