v4.3.0 additions #462
v4.3.0 additions #462
Conversation
Updated toml to include sphinx for dev
Addition of the cumulative sum tally modifier
Addition of support for OpenMC compiled sources
…odifier. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Removing tally factors, integrating volume and mass divisors into raw config yaml
Benchmarks overview documentation
Introduce a "force" option in the raw processing in order to allow for results overriding
Openmc warnings fix
…-more-customization-of-benchmarks-batch-submission 245 feature request allow for more customization of benchmarks batch submission
Add rcr fe+ss
Add isis bench
WalkthroughIntroduces new run modes and job-submission plumbing (local/job/global_job) with per-code templates; adds GitLab input fetching; extends raw processing with a force mode; consolidates OpenMC cell data and adds tally modifiers (cumulative_sum, gaussian_broadening, volume, mass); adds Sphere pivot table; updates CLI, resources, docs, CI, and tests; adds new benchmarks/configs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as jade CLI
participant App as JadeApp
participant Bench as BenchmarkRunFactory
participant Run as BenchmarkRun
participant Launch as launch_global_jobs
User->>CLI: jade --run
CLI->>App: run_benchmarks(testing=?)
App->>Bench: run()
Bench-->>App: list of (code, commands, folder)
App->>App: if run_mode == GLOBAL_JOB
App->>Launch: launch_global_jobs(commands, env_vars, test)
Launch-->>App: list of job ids or scripts
App-->>CLI: return jobs or None
sequenceDiagram
autonumber
actor User
participant CLI as jade CLI
participant App as JadeApp
participant Raw as RawProcessor
participant FS as Filesystem
User->>CLI: jade --raw [force]
CLI->>App: raw_process(force=bool, subset=?)
App->>FS: discover successful sims
App->>Raw: process(sim, modifiers,...)
Raw->>Raw: if VOLUME/MASS, inject cell volumes/masses
Raw->>FS: write CSVs
App-->>CLI: done
sequenceDiagram
autonumber
actor User
participant App as JadeApp
participant Fetch as fetch_iaea_inputs
participant GL as fetch_f4e_inputs
User->>App: update_inputs()
App->>Fetch: fetch IAEA data
Fetch-->>App: install paths
App->>App: read F4E_GITLAB_TOKEN
App->>GL: if token -> fetch F4E data
GL-->>App: install paths
App-->>User: inputs updated
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
alexvalentine94
left a comment
alexvalentine94
left a comment
There was a problem hiding this comment.
Great work all! 👏 🥕
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/jade/config/paths_tree.py (1)
135-142: Migration needed for renamedexe_cfgdirectoryChanging the path to
cfg/exe_configbreaks every pre-existing JADE installation where the folder is still namedcfg/exe_cfg; all executable configs become invisible after the upgrade. Please either provide automatic migration or keep backward compatibility by falling back to the old path when the new one doesn’t exist.src/jade/config/run_config.py (1)
214-222: Guard access tocode_job_templatefor backward compatibility.
cfg["code_job_template"]assumes every env YAML now defines that mapping. Existing installs without the new key will crash withKeyError. Please fallback withcfg.get("code_job_template")(and handleNone) so legacy configs still load.docs/source/documentation/benchdesc/cmodel.rst (1)
80-84: Minor typo: missing space"the ITERtokamak" → "the ITER tokamak".
- the ITERtokamak for comprehensive nuclear analyses”. + the ITER tokamak for comprehensive nuclear analyses”.tests/app/test_fetch.py (1)
9-67: Handle empty F4E_GITLAB_TOKEN valuesIf
F4E_GITLAB_TOKENis defined but empty (e.g., masked secrets in CI), the skip guard doesn’t trigger and the test runs with an invalid token, producing a false failure. Treat falsy values as missing so the test skips instead of hard-failing, and align the assertion accordingly.-# By default this should set the token to None if not found -F4E_GITLAB_TOKEN = os.getenv("F4E_GITLAB_TOKEN") +# By default this should set the token to None if not found or empty +F4E_GITLAB_TOKEN = os.getenv("F4E_GITLAB_TOKEN") or None … -@pytest.mark.skipif(F4E_GITLAB_TOKEN is None, reason="No token found") +@pytest.mark.skipif(not F4E_GITLAB_TOKEN, reason="No token found") def test_fetch_f4e_inputs(tmpdir): - assert F4E_GITLAB_TOKEN is not None + assert F4E_GITLAB_TOKENsrc/jade/post/sim_output.py (1)
308-334: Fix type hint: return type shows 4 values but implementation returns 3.The type hint at line 311 indicates four return values, but the implementation at line 334 returns only three values (file1, file2, file3), consistent with the updated OpenMCSimOutput flow.
Apply this diff:
@staticmethod def retrieve_file( results_path: PathLike, - ) -> tuple[PathLike, PathLike, PathLike | None, PathLike | None]: + ) -> tuple[PathLike, PathLike, PathLike | None]:src/jade/run/benchmark.py (1)
474-513: Fix return type hint mismatch for BenchmarkRun.runImplementation returns (code, commands, folder), but the hint lacks code. Align the annotation to avoid confusion and mypy errors.
- def run(self) -> list[tuple[list[str] | None, PathLike]]: + def run(self) -> list[tuple[CODE, list[str] | None, PathLike]]:
🧹 Nitpick comments (12)
docs/source/usage/postprocessing.rst (1)
22-27: Document the python -m variant of the force command.The preceding section shows both
jade --rawandpython -m jade --raw. Please mirror that here so users running viapython -mknow they can callpython -m jade --raw force.docs/source/usage/installation.rst (1)
101-106: Add copy-pastable token export example.Consider adding an explicit snippet so users can set the variable without guessing the syntax.
In case the user possesses a valid access token for the F4E GitLab, they can add an environment variable to their system named `F4E_GITLAB_TOKEN` with such token. JADE will automatically fetch inputs also from the F4E GitLab. Alternatively, the token can be added later and additional inputs can be recovered using the JADE utility: + | ``export F4E_GITLAB_TOKEN="<your-token>"`` (Linux/macOS) + | ``setx F4E_GITLAB_TOKEN "<your-token>"`` (Windows) + | ``python -m jade.utilities --fetch``tests/dummy_structure/cfg/env_vars_cfg.yml (1)
10-13: Update comments to match new run modes and paths
- Include global_job in run_mode comment.
- Fix path note: exe_config (not exec_config).
-# run mode is either "local" to run locally or "job" to submit to a cluster +# run mode is one of: "local" (run locally), "job" (submit per-case jobs), "global_job" (single global submission) -# path can be expressed as relative to cfg/exec_config or absolute +# path can be expressed as relative to cfg/exe_config or absolutesrc/jade/resources/default_cfg/benchmarks_pp/excel/RCR-Fe+Ni.yaml (1)
8-9: Optional: Add space after commas in lists for consistency.YAMLlint suggests adding a space after commas in list literals for improved readability. While the YAML is valid as-is, adding spaces would align with common style conventions.
Example diff for line 8:
- value: ['Value','Error'] + value: ['Value', 'Error']Apply similar changes to lines 9, 18, 19, 36, and 55.
Also applies to: 18-19, 36-36, 55-55
tests/app/test_app.py (1)
52-53: Rename unused loop variableRuff rightly flags
codeas unused here. Rename it to_code(or drop it entirely) to satisfy the linter without changing behaviour.- for code, lib in bench.run: + for _code, lib in bench.run:src/jade/app/app.py (2)
98-112: F4E inputs update flow is fine; consider downgrading missing-token log to debugNo token is a normal scenario; INFO may be noisy. Optional: log at DEBUG to keep logs cleaner.
160-166: Suppress unused loop variable warning (bench_name)bench_name isn’t used here. Use underscore to clarify intent and silence linters.
- for bench_name, cfg in self.run_cfg.benchmarks.items(): + for _, cfg in self.run_cfg.benchmarks.items():src/jade/run/benchmark.py (5)
904-912: Avoid spawning a subprocess for user lookup; use getpass.getuser()This removes an unnecessary process and quiets S607.
- # store cwd to get back to it after job submission - user = subprocess.run("whoami", capture_output=True).stdout.decode("utf-8").strip() + # store cwd to get back to it after job submission + import getpass + user = getpass.getuser()Additionally, add the import near the top if you prefer grouping imports.
246-253: _submit_job: return type and behavior in test modeDocstring says None, but in test mode you return contents (str). Update the annotation/docstring accordingly for clarity.
- ) -> None: + ) -> str | None: @@ - """Submits a job script to the users batch system for running in parallel. + """Submits a job script to the users batch system for running in parallel. + In test mode, returns the generated job script contents.
274-291: MCNP run command includes an empty argument when tasks == ""An empty string ends up as an extra argument. Filter falsy items before returning.
- run_command = [executable, inputstring, outputstring, xsstring, tasks] + run_command = [x for x in [executable, inputstring, outputstring, xsstring, tasks] if x]
967-974: Redundant chdir back to cwdYou never chdir away from cwd in launch_global_jobs; os.chdir(cwd) is a no-op. Remove to simplify.
- # return to the original directory - os.chdir(cwd) + # no directory change performed; nothing to revert
190-209: SingleRun.run always returns False in non-test modesflagnotrun is never set True; function returns False regardless of outcome. If unused, consider returning None. If used, set it meaningfully or return success status.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (95)
docs/source/documentation/computational_overview.csvis excluded by!**/*.csvdocs/source/documentation/exp_overview.csvis excluded by!**/*.csvdocs/source/img/benchmarks/RCR-Fe+Ni_1.pngis excluded by!**/*.pngdocs/source/img/benchmarks/RCR-Fe+Ni_2.pngis excluded by!**/*.pngdocs/source/img/benchmarks/RCR-SS_1.pngis excluded by!**/*.pngdocs/source/img/benchmarks/RCR-SS_2.pngis excluded by!**/*.pngdocs/source/img/benchmarks/isis-fig1.pngis excluded by!**/*.pngdocs/source/img/benchmarks/isis-fig2.pngis excluded by!**/*.pngdocs/source/img/benchmarks/isis-fig3.pngis excluded by!**/*.pngdocs/source/img/benchmarks/isis-fig4.pngis excluded by!**/*.pngdocs/source/img/benchmarks/isis-fig5.pngis excluded by!**/*.pngtests/dummy_structure/benchmark_templates/ITER_1D/ITER_1D/openmc/libsource.sois excluded by!**/*.sotests/dummy_structure/raw_data/_exp_-_exp_/ISIS-800MeV-C/ISIS-800MeV-C_Ct Reaction rate Ct.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/ISIS-800MeV-C/ISIS-800MeV-C_Fe Reaction rate Fe.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Fe Coarse neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Fe Coarse neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Fe Neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Fe Neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Fe Total Reaction Rate In.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Fe Total Reaction Rate Ni.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Ni Coarse neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Ni Coarse neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Ni Neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Ni Neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Ni Total Reaction Rate In.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-Fe+Ni/RCR-Ni Total Reaction Rate Ni.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_RI Total Reaction Rate Nb.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_RI Total Reaction Rates Au.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_RI Total Reaction Rates Cu.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_RI Total Reaction Rates Ni (RI).csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_RI Total Reaction Rates Ta.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Coarse neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Coarse neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Total Reaction Rate In1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Total Reaction Rate In2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_exp_-_exp_/RCR-SS/RCR-SS_SR Total Reaction Rate Ni (SR).csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/ISIS-800MeV-C/ISIS-800MeV-C_Ct Reaction rate Ct.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/ISIS-800MeV-C/ISIS-800MeV-C_Fe Reaction rate Fe.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Coarse neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Coarse neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Cumulative Reaction Rate In.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Cumulative Reaction Rate Ni.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Total Reaction Rate In.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe Total Reaction Rate Ni.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Coarse neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Coarse neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Cumulative Reaction Rate In.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Cumulative Reaction Rate Ni.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Total Reaction Rate In.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni Total Reaction Rate Ni.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rate Nb.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Au pos 1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Au pos 2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Au pos 3.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Au pos 4.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Cu pos 1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Cu pos 2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Cu pos 3.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Cu pos 4.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ni (RI) pos 1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ni (RI) pos 2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ni (RI) pos 3.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ni (RI) pos 4.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ta pos 1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ta pos 2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ta pos 3.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Cumulative Reaction Rates Ta pos 4.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Total Reaction Rate Nb.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Total Reaction Rates Au.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Total Reaction Rates Cu.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Total Reaction Rates Ni (RI).csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI Total Reaction Rates Ta.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Coarse neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Coarse neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Cumulative Reaction Rate In1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Cumulative Reaction Rate In2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Cumulative Reaction Rate Ni (SR).csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Neutron flux HPD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Neutron flux SSD.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Total Reaction Rate In1.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Total Reaction Rate In2.csvis excluded by!**/*.csvtests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR Total Reaction Rate Ni (SR).csvis excluded by!**/*.csvtests/dummy_structure/simulations/_mcnp_-_FENDL 3.2c_/ISIS-800MeV-C/ISIS-800MeV-C_Ct/ISIS-800MEV_C_Ct.ois excluded by!**/*.otests/dummy_structure/simulations/_mcnp_-_FENDL 3.2c_/ISIS-800MeV-C/ISIS-800MeV-C_Fe/ISIS-800MEV_C_Fe.ois excluded by!**/*.otests/dummy_structure/simulations/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Fe/RCR-CF_FE_SR_32c.ois excluded by!**/*.otests/dummy_structure/simulations/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/RCR-Ni/RCR-CF_Ni_SR_32c.ois excluded by!**/*.otests/dummy_structure/simulations/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_RI/RCR-CF_SS_RI_32c.ois excluded by!**/*.otests/dummy_structure/simulations/_mcnp_-_FENDL 3.2c_/RCR-SS/RCR-SS_SR/RCR-CF_SS_SR_32c.ois excluded by!**/*.otests/dummy_structure/simulations/_openmc_-_FENDL 3.2b_/ITER_1D/ITER_1D/tallies.outis excluded by!**/*.out
📒 Files selected for processing (104)
.coveragerc(1 hunks).github/ISSUE_TEMPLATE/improve-documentation.md(1 hunks).github/ISSUE_TEMPLATE/new_benchmark.md(1 hunks).github/PULL_REQUEST_TEMPLATE.md(0 hunks).github/PULL_REQUEST_TEMPLATE/bug_fix_template.md(1 hunks).github/PULL_REQUEST_TEMPLATE/new_benchmark.md(1 hunks).github/PULL_REQUEST_TEMPLATE/new_feature_template.md(1 hunks).github/workflows/pytest.yml(5 hunks).gitignore(1 hunks)docs/source/dev/insertbenchmarks.rst(6 hunks)docs/source/dev/pp_gallery.rst(1 hunks)docs/source/documentation/benchdesc/ISIS-800MeV-C.rst(1 hunks)docs/source/documentation/benchdesc/cmodel.rst(1 hunks)docs/source/documentation/benchdesc/itercylSDDR.rst(2 hunks)docs/source/documentation/benchdesc/rcr/rcr-fe+ni.rst(1 hunks)docs/source/documentation/benchdesc/rcr/rcr-ss.rst(1 hunks)docs/source/documentation/benchdesc/sphereSDDR.rst(2 hunks)docs/source/documentation/benchmarks.rst(3 hunks)docs/source/usage/installation.rst(3 hunks)docs/source/usage/postprocessing.rst(1 hunks)docs/source/usage/user_configuration.rst(1 hunks)docs/source/usage/utilities.rst(1 hunks)pyproject.toml(2 hunks)src/jade/__main__.py(2 hunks)src/jade/app/app.py(6 hunks)src/jade/app/fetch.py(5 hunks)src/jade/config/excel_config.py(1 hunks)src/jade/config/paths_tree.py(1 hunks)src/jade/config/raw_config.py(1 hunks)src/jade/config/run_config.py(7 hunks)src/jade/helper/constants.py(1 hunks)src/jade/helper/openmc.py(11 hunks)src/jade/post/excel_routines.py(4 hunks)src/jade/post/manipulate_tally.py(4 hunks)src/jade/post/plotter.py(4 hunks)src/jade/post/raw_processor.py(2 hunks)src/jade/post/sim_output.py(3 hunks)src/jade/resources/default_cfg/batch_templates/LLtemplate.cmd(0 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/ASPIS-Fe88.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/ASPIS-PCA-Replica.yaml(5 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/C-Model.yaml(10 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-BKT.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-HCPB.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-SDDR.yaml(6 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-SS.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-SiC.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-W.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/FNS-TOF.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/HCPB_TBM_1D.yaml(5 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/IPPE-CF.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/IPPE-DT.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/ISIS-800MeV-C.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/ITER_1D.yaml(5 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/ITER_Cyl_SDDR.yaml(5 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/Oktavian.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/RCR-Fe+Ni.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/RCR-SS.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/Simple_Tokamak.yaml(4 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/Sphere.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/SphereSDDR.yaml(7 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/TUD-FNG.yaml(4 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/TUD-Fe.yaml(6 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/TUD-W.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/Tiara-BC.yaml(2 hunks)src/jade/resources/default_cfg/benchmarks_pp/atlas/WCLL_TBM_1D.yaml(5 hunks)src/jade/resources/default_cfg/benchmarks_pp/excel/ISIS-800MeV-C.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/excel/RCR-Fe+Ni.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/excel/RCR-SS.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/excel/Sphere.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/excel/SphereSDDR.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/raw/mcnp/ISIS-800MeV-C.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/raw/mcnp/RCR-Fe+Ni.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/raw/mcnp/RCR-SS.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/raw/openmc/ITER_1D.yaml(1 hunks)src/jade/resources/default_cfg/benchmarks_pp/raw/openmc/Simple_Tokamak.yaml(2 hunks)src/jade/resources/default_cfg/env_vars_cfg.yml(1 hunks)src/jade/resources/default_cfg/exe_config/d1s_config.sh(0 hunks)src/jade/resources/default_cfg/exe_config/d1s_template.sh(1 hunks)src/jade/resources/default_cfg/exe_config/mcnp_config.sh(0 hunks)src/jade/resources/default_cfg/exe_config/mcnp_template.sh(1 hunks)src/jade/resources/default_cfg/exe_config/openmc_config.sh(0 hunks)src/jade/resources/default_cfg/exe_config/openmc_template.sh(1 hunks)src/jade/resources/default_cfg/exe_config/serpent_config.sh(0 hunks)src/jade/resources/default_cfg/exe_config/serpent_template.sh(1 hunks)src/jade/run/benchmark.py(18 hunks)src/jade/run/unix.py(0 hunks)tests/app/test_app.py(3 hunks)tests/app/test_fetch.py(2 hunks)tests/config/test_run_config.py(3 hunks)tests/dummy_structure/benchmark_templates/ITER_1D/ITER_1D/openmc/geometry.xml(1 hunks)tests/dummy_structure/benchmark_templates/ITER_1D/ITER_1D/openmc/materials.xml(1 hunks)tests/dummy_structure/cfg/batch_templates/LLtemplate.cmd(0 hunks)tests/dummy_structure/cfg/benchmarks_pp/excel/Sphere.yaml(2 hunks)tests/dummy_structure/cfg/env_vars_cfg.yml(1 hunks)tests/dummy_structure/cfg/exe_config/d1s_config.sh(0 hunks)tests/dummy_structure/cfg/exe_config/d1s_template.sh(1 hunks)tests/dummy_structure/cfg/exe_config/mcnp_config.sh(0 hunks)tests/dummy_structure/cfg/exe_config/mcnp_template.sh(1 hunks)tests/dummy_structure/cfg/exe_config/openmc_config.sh(0 hunks)tests/dummy_structure/cfg/exe_config/openmc_template.sh(1 hunks)tests/dummy_structure/cfg/exe_config/serpent_config.sh(0 hunks)tests/dummy_structure/cfg/exe_config/serpent_template.sh(1 hunks)tests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/ISIS-800MeV-C/metadata.json(1 hunks)tests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/metadata.json(1 hunks)
💤 Files with no reviewable changes (12)
- .github/PULL_REQUEST_TEMPLATE.md
- src/jade/resources/default_cfg/exe_config/serpent_config.sh
- tests/dummy_structure/cfg/exe_config/serpent_config.sh
- src/jade/resources/default_cfg/batch_templates/LLtemplate.cmd
- tests/dummy_structure/cfg/exe_config/mcnp_config.sh
- src/jade/run/unix.py
- src/jade/resources/default_cfg/exe_config/openmc_config.sh
- tests/dummy_structure/cfg/batch_templates/LLtemplate.cmd
- src/jade/resources/default_cfg/exe_config/d1s_config.sh
- tests/dummy_structure/cfg/exe_config/openmc_config.sh
- src/jade/resources/default_cfg/exe_config/mcnp_config.sh
- tests/dummy_structure/cfg/exe_config/d1s_config.sh
🧰 Additional context used
🧬 Code graph analysis (11)
tests/app/test_fetch.py (1)
src/jade/app/fetch.py (2)
fetch_f4e_inputs(178-217)fetch_iaea_inputs(147-175)
src/jade/app/app.py (6)
src/jade/app/fetch.py (2)
fetch_f4e_inputs(178-217)fetch_iaea_inputs(147-175)src/jade/config/paths_tree.py (1)
PathsTree(14-123)src/jade/config/raw_config.py (2)
ConfigRawProcessor(11-34)from_yaml(23-34)src/jade/config/run_config.py (4)
RunConfig(21-109)RunMode(515-520)create(369-408)from_yaml(193-222)src/jade/run/benchmark.py (11)
run(142-209)run(474-513)BenchmarkRunFactory(853-879)launch_global_jobs(925-974)create(342-380)create(855-879)code(75-76)code(271-272)code(300-301)code(322-323)code(336-337)src/jade/config/status.py (1)
get_successful_simulations(136-151)
tests/app/test_app.py (3)
tests/run/test_benchmark.py (1)
env_vars(277-289)src/jade/config/run_config.py (2)
RunMode(515-520)RunConfig(21-109)src/jade/run/benchmark.py (8)
code(75-76)code(271-272)code(300-301)code(322-323)code(336-337)run(142-209)run(474-513)continue_run(411-430)
src/jade/post/manipulate_tally.py (2)
src/jade/helper/openmc.py (2)
volumes(113-125)masses(141-151)src/jade/config/raw_config.py (1)
TallyModOption(82-103)
src/jade/post/raw_processor.py (1)
src/jade/config/raw_config.py (2)
ConfigRawProcessor(11-34)TallyModOption(82-103)
tests/config/test_run_config.py (3)
src/jade/helper/constants.py (1)
CODE(4-9)src/jade/config/run_config.py (2)
RunMode(515-520)EnvironmentVariables(128-222)src/jade/helper/errors.py (1)
ConfigError(1-6)
src/jade/run/benchmark.py (4)
tests/run/test_benchmark.py (1)
env_vars(277-289)src/jade/config/run_config.py (2)
RunMode(515-520)EnvironmentVariables(128-222)src/jade/helper/constants.py (1)
CODE(4-9)src/jade/helper/errors.py (1)
ConfigError(1-6)
src/jade/__main__.py (1)
src/jade/app/app.py (1)
raw_process(194-256)
src/jade/config/run_config.py (3)
tests/run/test_benchmark.py (1)
env_vars(277-289)src/jade/helper/constants.py (1)
CODE(4-9)src/jade/helper/errors.py (1)
ConfigError(1-6)
src/jade/post/excel_routines.py (1)
src/jade/config/excel_config.py (1)
TableType(13-19)
src/jade/post/sim_output.py (1)
src/jade/helper/openmc.py (1)
OpenMCStatePoint(408-592)
🪛 Ruff (0.13.2)
tests/app/test_fetch.py
50-50: Possible hardcoded password assigned to argument: "access_token"
(S106)
src/jade/app/app.py
160-160: Loop control variable bench_name not used within loop body
(B007)
tests/app/test_app.py
52-52: Loop control variable code not used within loop body
Rename unused code to _code
(B007)
src/jade/post/manipulate_tally.py
254-254: Avoid specifying long messages outside the exception class
(TRY003)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
291-293: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Prefer TypeError exception for invalid type
(TRY004)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
305-305: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
src/jade/run/benchmark.py
262-262: subprocess call: check for execution of untrusted input
(S603)
420-420: subprocess call: check for execution of untrusted input
(S603)
822-824: Avoid specifying long messages outside the exception class
(TRY003)
906-906: Starting a process with a partial executable path
(S607)
910-912: Avoid specifying long messages outside the exception class
(TRY003)
916-916: Use explicit conversion flag
Replace with conversion flag
(RUF010)
971-971: subprocess call: check for execution of untrusted input
(S603)
src/jade/app/fetch.py
70-70: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
94-94: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
src/jade/__main__.py
46-46: Avoid specifying long messages outside the exception class
(TRY003)
src/jade/config/run_config.py
177-179: Avoid specifying long messages outside the exception class
(TRY003)
186-188: Avoid specifying long messages outside the exception class
(TRY003)
src/jade/post/excel_routines.py
142-144: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🪛 YAMLlint (1.37.1)
src/jade/resources/default_cfg/benchmarks_pp/raw/mcnp/RCR-SS.yaml
[warning] 18-18: too few spaces after comma
(commas)
[warning] 45-45: too few spaces after comma
(commas)
[warning] 45-45: too few spaces after comma
(commas)
[warning] 45-45: too few spaces after comma
(commas)
[warning] 56-56: too few spaces after comma
(commas)
[warning] 56-56: too few spaces after comma
(commas)
[warning] 56-56: too few spaces after comma
(commas)
[warning] 67-67: too few spaces after comma
(commas)
[warning] 67-67: too few spaces after comma
(commas)
[warning] 67-67: too few spaces after comma
(commas)
[warning] 97-97: too few spaces after comma
(commas)
[warning] 97-97: too few spaces after comma
(commas)
[warning] 97-97: too few spaces after comma
(commas)
[warning] 100-100: too few spaces after comma
(commas)
[warning] 100-100: too few spaces after comma
(commas)
[warning] 100-100: too few spaces after comma
(commas)
[warning] 103-103: too few spaces after comma
(commas)
[warning] 103-103: too few spaces after comma
(commas)
[warning] 103-103: too few spaces after comma
(commas)
[warning] 106-106: too few spaces after comma
(commas)
[warning] 106-106: too few spaces after comma
(commas)
[warning] 106-106: too few spaces after comma
(commas)
[warning] 136-136: too few spaces after comma
(commas)
[warning] 136-136: too few spaces after comma
(commas)
[warning] 136-136: too few spaces after comma
(commas)
[warning] 139-139: too few spaces after comma
(commas)
[warning] 139-139: too few spaces after comma
(commas)
[warning] 139-139: too few spaces after comma
(commas)
[warning] 142-142: too few spaces after comma
(commas)
[warning] 142-142: too few spaces after comma
(commas)
[warning] 142-142: too few spaces after comma
(commas)
[warning] 145-145: too few spaces after comma
(commas)
[warning] 145-145: too few spaces after comma
(commas)
[warning] 145-145: too few spaces after comma
(commas)
[warning] 175-175: too few spaces after comma
(commas)
[warning] 175-175: too few spaces after comma
(commas)
[warning] 175-175: too few spaces after comma
(commas)
[warning] 178-178: too few spaces after comma
(commas)
[warning] 178-178: too few spaces after comma
(commas)
[warning] 178-178: too few spaces after comma
(commas)
[warning] 181-181: too few spaces after comma
(commas)
[warning] 181-181: too few spaces after comma
(commas)
[warning] 181-181: too few spaces after comma
(commas)
[warning] 184-184: too few spaces after comma
(commas)
[warning] 184-184: too few spaces after comma
(commas)
[warning] 184-184: too few spaces after comma
(commas)
[warning] 214-214: too few spaces after comma
(commas)
[warning] 214-214: too few spaces after comma
(commas)
[warning] 214-214: too few spaces after comma
(commas)
[warning] 217-217: too few spaces after comma
(commas)
[warning] 217-217: too few spaces after comma
(commas)
[warning] 217-217: too few spaces after comma
(commas)
[warning] 220-220: too few spaces after comma
(commas)
[warning] 220-220: too few spaces after comma
(commas)
[warning] 220-220: too few spaces after comma
(commas)
[warning] 223-223: too few spaces after comma
(commas)
[warning] 223-223: too few spaces after comma
(commas)
[warning] 223-223: too few spaces after comma
(commas)
[warning] 234-234: too few spaces after comma
(commas)
[warning] 234-234: too few spaces after comma
(commas)
[warning] 234-234: too few spaces after comma
(commas)
src/jade/resources/default_cfg/env_vars_cfg.yml
[warning] 8-8: too many spaces after colon
(colons)
src/jade/resources/default_cfg/benchmarks_pp/raw/mcnp/RCR-Fe+Ni.yaml
[warning] 16-16: too few spaces after comma
(commas)
[warning] 51-51: too few spaces after comma
(commas)
[warning] 51-51: too few spaces after comma
(commas)
[warning] 51-51: too few spaces after comma
(commas)
src/jade/resources/default_cfg/benchmarks_pp/excel/RCR-Fe+Ni.yaml
[warning] 8-8: too few spaces after comma
(commas)
[warning] 9-9: too few spaces after comma
(commas)
[warning] 18-18: too few spaces after comma
(commas)
[warning] 19-19: too few spaces after comma
(commas)
[warning] 36-36: too few spaces after comma
(commas)
[warning] 55-55: too few spaces after comma
(commas)
🔇 Additional comments (40)
docs/source/usage/utilities.rst (1)
16-26: Doc updates look correct.Command invocation and utility descriptions read clearly and match the intended behavior.
docs/source/usage/installation.rst (3)
18-18: Activation command now matches the venv name.Switching to
source jade_venv/bin/activatealigns the activation path with the virtual environment that users just created. Nicely tightened guidance.
47-49: Good call-out on Ubuntu’s Tkinter prerequisite.Highlighting the need for
python3-tkbefore pulling in the UI extras will save Ubuntu users from runtime surprises.
95-97: Benchmarks fetch note is appreciated.Clarifying that the IAEA benchmark inputs download during initialization sets expectations about the required network access.
.gitignore (1)
28-28: Removal of tests/secrets.json is safe: no file or reference to tests/secrets.json exists in the repository.src/jade/config/excel_config.py (1)
19-19: LGTM!The addition of
SPHERE_PIVOTextends theTableTypeenum cleanly and aligns with the new sphere-pivot table rendering flow described in the AI summary.src/jade/resources/default_cfg/benchmarks_pp/excel/SphereSDDR.yaml (1)
8-8: LGTM!The change to
sphere_pivotcorrectly adopts the newTableType.SPHERE_PIVOTenum member and enables sphere-specific rendering behavior.src/jade/resources/default_cfg/benchmarks_pp/atlas/ASPIS-Fe88.yaml (1)
6-6: LGTM!The LaTeX formatting for the x-axis label is correctly applied and aligns with the repository-wide effort to standardize axis label rendering.
src/jade/resources/default_cfg/exe_config/mcnp_template.sh (2)
10-11: LGTM!The addition of the
# modulescomment provides a clear placeholder for module loading directives.
9-9: No duplicate SBATCH --ntasks directive
Only one occurrence was found; no consolidation needed.src/jade/resources/default_cfg/benchmarks_pp/atlas/ITER_Cyl_SDDR.yaml (1)
7-7: LGTM!The LaTeX formatting for all y-axis labels is correctly applied and provides consistent, professional rendering of units across the benchmark configuration.
Also applies to: 18-18, 30-30, 42-42, 54-54
src/jade/resources/default_cfg/benchmarks_pp/atlas/FNS-TOF.yaml (1)
10-11: LGTM!The LaTeX formatting for axis labels is correctly applied and aligns with the repository-wide standardization effort.
Also applies to: 30-30
src/jade/resources/default_cfg/benchmarks_pp/atlas/FNG-SiC.yaml (1)
6-6: LGTM!The LaTeX formatting for x-axis labels is correctly applied and maintains consistency with other atlas benchmark configurations.
Also applies to: 20-20
src/jade/resources/default_cfg/benchmarks_pp/atlas/IPPE-DT.yaml (1)
6-7: LGTM!The LaTeX formatting for both x-axis and y-axis labels is correctly applied, completing the consistent labeling style across the IPPE-DT benchmark configuration.
tests/dummy_structure/cfg/benchmarks_pp/excel/Sphere.yaml (1)
1-1: LGTM!The change from
pivottosphere_pivotaligns with the PR's introduction of theSpherePivotTableclass. The intentionally long sheet name ("A long name that will break the excel sheets:") appears to be a test case for handling/truncating long Excel sheet names.Also applies to: 11-11
src/jade/post/plotter.py (1)
312-312: LGTM!The
yscaleparameter is correctly implemented, with proper default and conditional Locator logic for log scales. This aligns with the PR's goal of adding configurable y-axis scaling for binned plots.Also applies to: 319-319, 358-358, 363-365
.github/workflows/pytest.yml (1)
32-32: LGTM!The upgrades and additions are appropriate:
setup-python@v6is the latest stable version.- Adding
[dev,ui]extras ensures UI-related dependencies are tested.F4E_GITLAB_TOKENsecret aligns with the PR's new GitLab input fetching feature.Also applies to: 57-57, 74-74, 85-85, 95-95
src/jade/resources/default_cfg/benchmarks_pp/atlas/ASPIS-PCA-Replica.yaml (1)
6-6: LGTM!The LaTeX-formatted axis labels are correctly escaped and will render properly in matplotlib plots. This improves the presentation quality of the plots.
Also applies to: 19-19, 32-32, 46-46, 60-60
docs/source/dev/pp_gallery.rst (1)
150-150: Fix the typo in the plot argument name.The documentation states
ysclaebut the correct argument name isyscale(as implemented insrc/jade/post/plotter.pyline 312 and 319).Apply this diff:
-* ``ysclae``: The scale of the y-axis. Every argument that could be passed to the matplotlib function +* ``yscale``: The scale of the y-axis. Every argument that could be passed to the matplotlib functionLikely an incorrect or invalid review comment.
src/jade/resources/default_cfg/benchmarks_pp/atlas/Tiara-BC.yaml (1)
6-7: LGTM: label formattingLaTeX-style labels look correct and consistently escaped.
Also applies to: 22-22
src/jade/resources/default_cfg/benchmarks_pp/excel/ISIS-800MeV-C.yaml (1)
1-11: Pivot config looks consistent; verify column-index renameConfig is coherent. Please confirm that using integer key 0 in change_col_names is expected by the pivot/ratio pipeline for labeling C/E.
Also applies to: 12-22
src/jade/resources/default_cfg/benchmarks_pp/atlas/C-Model.yaml (1)
7-7: LGTM: y-axis label normalizationLaTeX-formatted units are consistent and correct across sections.
Also applies to: 25-25, 42-42, 61-61, 78-78, 91-91, 109-109, 121-121, 133-133, 145-145
tests/dummy_structure/benchmark_templates/ITER_1D/ITER_1D/openmc/geometry.xml (1)
1-214: LGTM! Well-structured OpenMC geometry definition.The geometry XML is well-formed with 105 cells and 111 surfaces defining a detailed 1D cylindrical benchmark. All cells consistently reference valid surface IDs, and boundary conditions (reflective planes, vacuum cylinder) are appropriate for this axially symmetric configuration.
src/jade/resources/default_cfg/benchmarks_pp/excel/RCR-Fe+Ni.yaml (1)
1-68: LGTM! Comprehensive RCR-Fe+Ni benchmark configuration.The configuration correctly defines multiple comparison types (absolute, ratio, chi_squared) with appropriate table types and axis mappings. The structure aligns well with related benchmark configs in this PR.
docs/source/documentation/benchmarks.rst (3)
21-27: LGTM! Clear overview of benchmark repositories.The added overview section effectively explains where benchmarks are hosted (IAEA open benchmarks, private F4E GitLab, GitHub Project for under-development work) and provides helpful links for users.
33-33: LGTM! Improved table column proportions.The updated widths (60 30 30) better accommodate the benchmark tables compared to the previous allocation (70 10 10 10).
Also applies to: 40-40
95-97: LGTM! Added RCR benchmark documentation references.The new includes for rcr-ss.rst and rcr-fe+ni.rst expand the experimental benchmarks section, aligning with the new benchmark configs added in this PR.
src/jade/resources/default_cfg/benchmarks_pp/atlas/Sphere.yaml (1)
6-7: LGTM! Enhanced axis labels with LaTeX formatting.The updated axis labels use proper mathematical typesetting, which will render professionally in matplotlib plots. The LaTeX syntax is correct with properly escaped backslashes for YAML strings.
Also applies to: 20-21
docs/source/documentation/benchdesc/ISIS-800MeV-C.rst (1)
1-73: LGTM! Comprehensive ISIS benchmark documentation.The documentation provides detailed information about the ISIS experiment, including experimental setup, geometry, detector positions, and tallies. The structure is clear and consistent with other benchmark descriptions in JADE. The prominent note about distribution restrictions appropriately warns users about licensing constraints.
src/jade/post/raw_processor.py (1)
8-8: LGTM! Added necessary import for TallyModOption.The import is required for the new special-case handling of VOLUME and MASS tally modifications introduced below.
src/jade/resources/default_cfg/benchmarks_pp/raw/openmc/Simple_Tokamak.yaml (1)
56-121: LGTM! Consistent volume normalization pattern applied.The two-step transformation pattern (volume operation followed by replace/add_column) is consistently applied across flux, spectra, tritium production, and DPA tallies. This aligns with the special-case handling added in raw_processor.py, which will populate the volume data from cell_data at runtime.
src/jade/resources/default_cfg/benchmarks_pp/atlas/ISIS-800MeV-C.yaml (1)
1-28: LGTM! Well-structured ISIS benchmark configuration.The configuration correctly defines C/E ratio plots for the C12(n,2n)C11 reaction rates through concrete (Ct) and iron (Fe) shields. The plot parameters (ce_limits, style, axes) are appropriate and align with the experimental setup described in the companion documentation file.
docs/source/documentation/benchdesc/itercylSDDR.rst (1)
63-63: LGTM: Documentation accuracy improved.The caption update and clarifying note improve the documentation's accuracy by distinguishing the conservative scenario from the official ITER SA2 scenario.
Also applies to: 98-101
docs/source/documentation/benchdesc/sphereSDDR.rst (1)
48-50: LGTM: Documentation clarity improved.The caption update, clarifying note, and narrative refinement improve the documentation's accuracy and clarity, consistent with the parallel updates in itercylSDDR.rst.
Also applies to: 81-91
src/jade/resources/default_cfg/benchmarks_pp/excel/RCR-SS.yaml (1)
1-78: LGTM: Benchmark configuration is well-structured.The new RCR-SS benchmark configuration defines clear comparison and table types for neutron flux and reaction-rate comparisons, consistent with similar benchmark configurations added in this PR.
docs/source/documentation/benchdesc/rcr/rcr-ss.rst (1)
1-329: LGTM: Comprehensive benchmark documentation.The new RCR Stainless Steel Block Experiment documentation is thorough, well-structured, and follows the established pattern for similar benchmark documentation in the project.
src/jade/resources/default_cfg/benchmarks_pp/atlas/Simple_Tokamak.yaml (1)
6-7: LGTM: Axis labels standardized with LaTeX formatting.The axis label updates improve presentation consistency across benchmark configurations without affecting functionality.
Also applies to: 23-24, 41-42, 58-59
docs/source/documentation/benchdesc/rcr/rcr-fe+ni.rst (1)
1-161: LGTM: Comprehensive benchmark documentation.The new RCR Iron/Nickel Sphere Experiment documentation is thorough, well-structured, and follows the established pattern for similar benchmark documentation in the project.
src/jade/app/app.py (1)
171-177: Return type behavior is correct for GLOBAL_JOB; confirm downstream expectationsrun_benchmarks now returns a list[str] only in GLOBAL_JOB. Ensure callers/tests don’t assume a value for other run modes.
src/jade/run/benchmark.py (1)
515-539: Type consistency across _run_sub_benchmarks and _run_single_runans can be None or list[str]. Ensure the declared return type of _run_sub_benchmarks and callers reflect this, and that downstream (GLOBAL_JOB only) doesn’t pass None to launch_global_jobs.
Also applies to: 540-554
| - [ ] I have added the required YAML files. | ||
| - [ ] I have updated the documentation to include the benchmark description. | ||
| - [ ] I have updated the documentation summary table which gives an overview of the implemented benchmark. | ||
| - [ ] Have the benchmark inputs been added to the appropriate respository. | ||
| - [ ] My changes generate no new warnings. No newline at end of file |
There was a problem hiding this comment.
Fix typo and punctuation in checklist item.
“Respository” is misspelled and the sentence reads awkwardly without a question mark.
-- [ ] Have the benchmark inputs been added to the appropriate respository.
+- [ ] Have the benchmark inputs been added to the appropriate repository?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [ ] I have added the required YAML files. | |
| - [ ] I have updated the documentation to include the benchmark description. | |
| - [ ] I have updated the documentation summary table which gives an overview of the implemented benchmark. | |
| - [ ] Have the benchmark inputs been added to the appropriate respository. | |
| - [ ] My changes generate no new warnings. | |
| - [ ] I have added the required YAML files. | |
| - [ ] I have updated the documentation to include the benchmark description. | |
| - [ ] I have updated the documentation summary table which gives an overview of the implemented benchmark. | |
| - [ ] Have the benchmark inputs been added to the appropriate repository? | |
| - [ ] My changes generate no new warnings. |
🤖 Prompt for AI Agents
In .github/PULL_REQUEST_TEMPLATE/new_benchmark.md around lines 27 to 31, fix the
typo and punctuation in the checklist item by changing "respository" to
"repository" and append a question mark to make it a proper question (e.g.,
"Have the benchmark inputs been added to the appropriate repository?"); ensure
the line follows the same checkbox format as the other items.
| tmpdirname = tempfile.gettempdir() | ||
| tmp_zip = os.path.join(tmpdirname, os.path.basename(url)) | ||
| tmp_zip = os.path.join(tmpdirname, dest) | ||
| extracted_folder = os.path.join(tmpdirname, "extracted") | ||
| with open(tmp_zip, "wb") as f: | ||
| f.write(response.content) | ||
| f.write(zip_content) | ||
| # Extract the zip file | ||
| with zipfile.ZipFile(tmp_zip, "r") as zip_ref: | ||
| main_folder_name = zip_ref.namelist()[0] | ||
| zip_ref.extractall(extracted_folder) | ||
|
|
||
| return extracted_folder | ||
| return Path(extracted_folder, main_folder_name) | ||
|
|
There was a problem hiding this comment.
Avoid shared temp filenames/directories in _extract_zip.
tmp_zip is always /tmp/<dest> and the extraction root is the fixed /tmp/extracted. Concurrent or back‑to‑back fetches will trample each other’s work, leaving partial installs or leaking files. Generate unique paths (e.g., tempfile.NamedTemporaryFile(delete=False) plus tempfile.mkdtemp()) and clean them up after use to keep fetches isolated.
🤖 Prompt for AI Agents
In src/jade/app/fetch.py around lines 56 to 67, the function writes to fixed
paths (/tmp/<dest> and /tmp/extracted) which causes race conditions and file
clobbering; change the implementation to create a unique temporary zip file
(e.g., tempfile.NamedTemporaryFile(delete=False) or tempfile.mkstemp()) and a
unique temporary extraction directory (tempfile.mkdtemp()), write the zip
content to the temp file, extract into the temp dir, return the extracted main
folder path, and ensure the temp zip file (and any other ephemeral resources you
create) are removed after extraction (but do not delete the returned extracted
folder), so concurrent fetches are isolated and temporary files are cleaned up.
| gl = gitlab.Gitlab(url=url, private_token=authorization_token, ssl_verify=False) | ||
| try: | ||
| gl.auth() | ||
| except gitlab.exceptions.GitlabAuthenticationError: | ||
| logging.error("Gitlab authentication failed") | ||
| return False | ||
|
|
There was a problem hiding this comment.
Do not disable TLS verification when talking to GitLab.
Forcing ssl_verify=False drops certificate checks, so access tokens and payloads can be intercepted. Please restore verification (default True) and, if you need a custom CA bundle, make it configurable instead of disabling TLS validation.
🧰 Tools
🪛 Ruff (0.13.2)
94-94: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🤖 Prompt for AI Agents
In src/jade/app/fetch.py around lines 90 to 96, the GitLab client is created
with ssl_verify=False which disables TLS certificate validation; change this to
use the default certificate verification (remove or set ssl_verify=True) and
make the CA bundle configurable instead of disabling validation (e.g., accept a
config parameter or environment variable for a custom CA bundle path and pass it
to the GitLab client or requests adapter). Ensure any config fallback defaults
to True/verified and do not hardcode ssl_verify=False; keep the existing
authentication try/except intact.
| # select the correct project | ||
| found = False | ||
| for project in gl.projects.list(): | ||
| if path == project.path_with_namespace: | ||
| found = True | ||
| break | ||
| if not found: | ||
| logging.error(f"Successful authentication but project {path} not found") | ||
| return False | ||
|
|
||
| binary = project.repository_archive(sha=branch, format="zip") |
There was a problem hiding this comment.
Look up the GitLab project directly instead of paging blindly.
gl.projects.list() returns the first page (default 20). Repos beyond that page are invisible, so valid paths fail with “project not found.” Use gl.projects.get(path_with_namespace) (or a properly paginated search) so any project can be fetched.
🤖 Prompt for AI Agents
In src/jade/app/fetch.py around lines 97 to 107, the code pages through
gl.projects.list() (default first 20) and can miss projects; replace the blind
list loop with a direct lookup using gl.projects.get(path_with_namespace) (or
use the paginated API properly). Call gl.projects.get(path) inside a try/except
to handle GitLabGetError/NotFound, log the error or "project not found" and
return False on failure, and only call project.repository_archive(...) when the
project is successfully retrieved.
| if self.code_job_template is not None and self.exe_cfg_root is not None: | ||
| for code, path in self.code_job_template.items(): | ||
| if not os.path.isabs(path) and self.exe_cfg_root is not None: | ||
| path = os.path.join(self.exe_cfg_root, path) | ||
| if not os.path.isfile(path): | ||
| raise ConfigError( | ||
| f"Job script template {path} not found, please check and re-run" | ||
| ) | ||
| code_job_template[code] = path | ||
| self.code_job_template = code_job_template | ||
|
|
There was a problem hiding this comment.
Preserve job templates even without exe_cfg_root.
Right now the normalization block only runs when exe_cfg_root is set, so providing absolute templates (or calling the dataclass with exe_cfg_root=None) silently clears code_job_template. Please always iterate self.code_job_template; only prefix relative paths when exe_cfg_root is available.
🧰 Tools
🪛 Ruff (0.13.2)
186-188: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/jade/config/run_config.py around lines 181 to 191, the current
normalization only runs when exe_cfg_root is set which causes code_job_template
to be cleared when exe_cfg_root is None; always iterate self.code_job_template
when it is not None, and only prefix relative paths with exe_cfg_root if
exe_cfg_root is available. For each code,path pair: if path is not absolute and
exe_cfg_root is not None then join exe_cfg_root with path, otherwise leave path
as-is; then validate with os.path.isfile and assign into the new
code_job_template dict so absolute templates are preserved even when
exe_cfg_root is None.
| <nuclide ao="2.40797e-05" name="Co59"/> | ||
| <nuclide ao="0.0072022" name="Ni58"/> | ||
| <nuclide ao="0.0027873" name="Ni60"/> | ||
| <nuclide ao="0.0001383" name="Ni61"/> | ||
| <nuclide ao="0.00038298" name="Ni62"/> | ||
| <nuclide ao="0.00012766" name="Ni64"/> | ||
| <nuclide ao="5.14751e-05" name="Cu63"/> | ||
| <nuclide ao="2.29646e-05" name="Cu65"/> | ||
| <nuclide ao="5.33578e-07" name="Zr90"/> | ||
| <nuclide ao="1.1636e-07" name="Zr91"/> | ||
| <nuclide ao="1.77859e-07" name="Zr92"/> |
There was a problem hiding this comment.
🧩 Analysis chain
Duplicate nuclides in material id="11" will break OpenMC
Nuclides Cu63/Cu65 are defined multiple times within the same material. OpenMC expects each nuclide once per material; duplicates cause errors or undefined behavior. Deduplicate and consolidate atomic fractions.
Run to list duplicates by material:
Also applies to: 248-256, 276-282, 290-296
🏁 Script executed:
#!/bin/bash
# Detect duplicate <nuclide name="..."> entries per <material>
awk '
/<material id="/{inmat=1; split($0,a,/id="|"/); mid=a[2]; delete seen}
inmat && /<nuclide /{
match($0,/name="[^"]+"/); name=substr($0,RSTART+6,RLENGTH-7);
if (seen[name]++) { printf("Duplicate nuclide %s in material %s\n", name, mid) }
}
/<\/material>/{inmat=0}
' tests/dummy_structure/benchmark_templates/ITER_1D/ITER_1D/openmc/materials.xmlLength of output: 1715
Deduplicate nuclide entries in materials id="13" and id="15"
- Material 13 contains multiple duplicate nuclides (e.g., Cu63, Cu65, C12…Sn124).
- Material 15 contains duplicate O16, O17, O18 entries.
OpenMC requires each nuclide only once per material; consolidate atomic fractions into single entries.
| #SBATCH --output=OUT_FILE | ||
| #SBATCH --error=ERROR_FILE | ||
| #SBATCH --mem-per-cpu=8000 | ||
| #SBATCH --time=16-48:00 |
There was a problem hiding this comment.
Verify the time format for the SBATCH directive.
The time format 16-48:00 is unusual. Slurm expects days-hours:minutes:seconds or hours:minutes:seconds. If this means 16 days and 48 hours, it should be 16-00:00 (16 days) plus 48 hours = 18-00:00. If it means 16 hours and 48 minutes, it should be 16:48:00.
Apply this diff if the intent is 16 hours and 48 minutes:
-#SBATCH --time=16-48:00
+#SBATCH --time=16:48:00Or apply this diff if the intent is 16 days (adjusting for the extra 48 hours to 18 days):
-#SBATCH --time=16-48:00
+#SBATCH --time=18-00:00📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #SBATCH --time=16-48:00 | |
| #SBATCH --time=16:48:00 |
| #SBATCH --time=16-48:00 | |
| #SBATCH --time=18-00:00 |
🤖 Prompt for AI Agents
In tests/dummy_structure/cfg/exe_config/d1s_template.sh around line 8, the
SBATCH time directive uses an invalid/ambiguous format "16-48:00"; replace it
with the correct Slurm format depending on intent: if you meant 16 hours 48
minutes change it to "16:48:00"; if you meant 16 days plus 48 hours, normalize
the total to days-hours by adding 48 hours (48h = 2 days) and use "18-00:00" for
18 days, or explicitly set days and remaining hours as "16-00:00" plus add 48h
elsewhere—update the SBATCH line accordingly.
| #!/bin/sh | ||
|
|
||
| #SBATCH --job-name="JADE" | ||
| #SBATCH --workdir=INITIAL_DIR |
There was a problem hiding this comment.
Fix invalid SBATCH options/time format
- Slurm uses --chdir (or -D), not --workdir.
- Time "16-48:00" is invalid (hours cannot be 48). Use either 16:48:00 (hh:mm:ss) or 16-00:00:00 (dd-hh:mm:ss).
Apply:
-#SBATCH --workdir=INITIAL_DIR
+#SBATCH --chdir=INITIAL_DIR
-#SBATCH --time=16-48:00
+#SBATCH --time=16:48:00Optional hardening:
#!/bin/sh
+set -euo pipefailAlso applies to: 8-8
🤖 Prompt for AI Agents
In tests/dummy_structure/cfg/exe_config/openmc_template.sh around line 4,
replace the invalid SBATCH option --workdir with Slurm's correct option --chdir
(or -D) and also fix any invalid time strings like "16-48:00" (hours cannot be
48): use a valid format such as "16:48:00" (hh:mm:ss) or "16-00:00:00"
(dd-hh:mm:ss) according to the intended meaning; apply the same corrections to
the occurrence at line 8 as noted.
| "jade_run_version": "3.1.0", | ||
| "library": "FENDL 3.2c", | ||
| "code": "mcnp", | ||
| "jade_version": "3.1.0", |
There was a problem hiding this comment.
Update JADE version metadata to 4.3.0.
These metadata entries still report jade_run_version/jade_version as 3.1.0, but this PR prepares the 4.3.0 release. Leaving the old values will mislabel the dataset and can break any tooling or assertions expecting the new release number. Please bump both fields to 4.3.0.
🤖 Prompt for AI Agents
In tests/dummy_structure/raw_data/_mcnp_-_FENDL
3.2c_/ISIS-800MeV-C/metadata.json around lines 4 to 7, the fields
"jade_run_version" and "jade_version" are set to "3.1.0" but should be updated
to "4.3.0"; edit those two entries to use "4.3.0" so the metadata reflects the
new release and avoids breaking tooling or assertions.
| "jade_run_version": "3.1.0", | ||
| "library": "FENDL 3.2c", | ||
| "code": "mcnp", | ||
| "jade_version": "3.1.0", |
There was a problem hiding this comment.
Align JADE version fields with the 4.3.0 release.
This metadata copy also still points to jade_run_version/jade_version 3.1.0. For the 4.3.0 rollout, bump both values so downstream consumers see the correct release tag.
🤖 Prompt for AI Agents
In tests/dummy_structure/raw_data/_mcnp_-_FENDL 3.2c_/RCR-Fe+Ni/metadata.json
around lines 4 to 7, the jade_run_version and jade_version fields are set to
"3.1.0" but need to reflect the 4.3.0 release; update both values from "3.1.0"
to "4.3.0" so downstream consumers see the correct release tag.
Bring latest developments to main for release v4.3.0.
Summary by CodeRabbit
New Features
--raw force.Documentation
Chores
Templates