Skip to content

Issue with FNG-HCPB post-processing#482

Open
jude-moo wants to merge 1 commit intoJADE-V-V:developingfrom
jude-moo:bugfix/FNG-HCPB_H3
Open

Issue with FNG-HCPB post-processing#482
jude-moo wants to merge 1 commit intoJADE-V-V:developingfrom
jude-moo:bugfix/FNG-HCPB_H3

Conversation

@jude-moo
Copy link
Collaborator

@jude-moo jude-moo commented Feb 10, 2026

The mass factor was being accounted for twice as it was contained within the scale factor also. This pull request removes the mass divisor to resolve this issue.

Summary by CodeRabbit

  • Refactor
    • Streamlined tritium activity processing in benchmark configurations by removing mass transformation steps from "Tritium activity" and "Tritium activity Pellet" calculation pipelines.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

Walkthrough

Removed two mass transformation steps from YAML benchmark configuration sections. The deletions simplify data processing pipelines in the "Tritium activity" and "Tritium activity Pellet" sections while preserving all remaining actions.

Changes

Cohort / File(s) Summary
YAML Configuration
src/jade/resources/default_cfg/benchmarks_pp/raw/openmc/FNG-HCPB.yaml
Removed two [- [mass, {}]] transformation steps from the "Tritium activity" and "Tritium activity Pellet" sections.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

bug

Suggested reviewers

  • dodu94

Poem

🐰 A mass of confusion has cleared from the way,
Two lines hopped away without delay,
The YAML now lightens, the config runs clean,
The simplest fix ever the rabbits have seen! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main issue: removing a duplicate mass transformation step in the FNG-HCPB post-processing configuration, which aligns with the PR's objective of fixing double-applied mass factors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉


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
Copy link
Member

dodu94 commented Feb 10, 2026

Thanks Jude, tests are failing due to an update of pandas to a major release (3.0).

Please do one of the following:

  1. cap the pandas version to pandas<3 and open an issue, we'll deal with it in the next hackathon

or

  1. fix the behaviour to be compatible with pandas 3.0 in the manipulate_tally module

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