Skip to content

Conversation

@digiacomo4
Copy link
Contributor

@digiacomo4 digiacomo4 commented Sep 30, 2025

Description

Please include a summary of the changes and the related issue (if applicable). Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #415

Type of change

Doc update

Summary by CodeRabbit

  • New Features
    • Fetch benchmark inputs from F4E GitLab using an access token (F4E_GITLAB_TOKEN).
    • Add cumulative_sum tally modifier with optional normalization.
    • Support OpenMC compiled sources (libsource.so) during input generation.
  • Refactor
    • Improved tally error propagation and energy group condensation.
    • Use non-interactive Matplotlib backend for headless plotting.
    • Expanded dosimetry libraries support.
  • Documentation
    • Added setup for IAEA/F4E input fetching, tokens, utilities (addrmode), OpenMC guidance, and detailed C-Model tallies.
  • Chores
    • Updated CI Python setup action; adjusted .gitignore; enhanced coverage HTML contexts; added dependencies.

@digiacomo4 digiacomo4 requested a review from dodu94 September 30, 2025 09:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 30, 2025

Walkthrough

Adds F4E GitLab-backed input fetching with token support, extends IAEA fetch plumbing, and updates app flow to optionally fetch from F4E. Introduces a new tally modifier (cumulative_sum) with revised error propagation, OpenMC compiled source handling, non-interactive plotting backend, config/docs updates, CI tweaks, and new/updated tests and fixtures.

Changes

Cohort / File(s) Summary
CI and project config
.coveragerc, .github/workflows/pytest.yml, .gitignore, pyproject.toml, src/jade/resources/default_cfg/env_vars_cfg.yml
Coverage HTML contexts enabled; GH action setup-python v6; env token F4E_GITLAB_TOKEN added to CI; ignore secrets.py; add runtime dep python-gitlab and Sphinx-related dev extras; add f4e_gitlab_token to default env vars config.
Documentation updates
docs/source/dev/insertbenchmarks.rst, docs/source/documentation/benchdesc/cmodel.rst, docs/source/usage/installation.rst, docs/source/usage/user_configuration.rst, docs/source/usage/utilities.rst
Expanded OpenMC guidance (Independent/CompiledSource), new tally modifier docs (cumulative_sum), detailed C-Model tallies, installation notes for IAEA/F4E inputs and token, config structure clarifications, utilities updates including --addrmode.
Input fetching feature
src/jade/app/app.py, src/jade/app/fetch.py
App now attempts IAEA fetch then optionally F4E fetch when token present; new GitLab fetch flow with auth, archive download, extraction, and standardized install; refactored common zip extraction; new helpers and public fetch_from_gitlab/fetch_f4e_inputs.
Post-processing: tally mods
src/jade/post/manipulate_tally.py, src/jade/config/raw_config.py, tests/post/test_manipulate_tally.py
Added cumulative_sum modifier with optional normalization and error propagation; improved error handling in groupby and condense groups; registered new enum CUMULATIVE_SUM; tests cover new logic and adjusted error propagation expectations.
OpenMC compiled source handling
src/jade/helper/openmc.py, tests/run/test_input.py, tests/dummy_structure/benchmark_templates/ITER_1D/ITER_1D/openmc/...
Detect/copy libsource.so and set CompiledSource in settings; create default settings if missing; new test validates source presence; added dummy OpenMC geometry.xml and materials.xml.
Plotting backend
src/jade/post/plotter.py
Forces Matplotlib "Agg" backend for headless operation.
Constants
src/jade/helper/constants.py
DOSIMETRY_LIBS updated to include "17c".
Config docstring
src/jade/config/run_config.py
Docstring tweaks marking parameters as optional.
Fetch tests
tests/app/test_fetch.py
Added F4E fetch tests with token handling and failure case; adjusted imports.
GUI tests stabilization
tests/gui/test_config_gui.py, tests/gui/test_post_config_gui.py
Mocked GUI roots (ttkthemes.ThemedTk/tkinter.Tk) and updated fixture scope/class name to avoid real GUI creation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant App as jade.app.app:update_inputs
  participant IAEA as IAEA Repo
  participant F4E as F4E GitLab
  Note over App: Start inputs update
  User->>App: update_inputs()
  App->>IAEA: fetch_iaea_inputs()
  alt IAEA fetch ok
    App-->>User: IAEA updated
  else IAEA fetch failed
    App-->>User: Log IAEA failure
    alt F4E_GITLAB_TOKEN present
      App->>F4E: fetch_f4e_inputs(token)
      alt F4E fetch ok
        App-->>User: F4E updated
      else F4E fetch failed
        App-->>User: Log F4E failure
      end
    else No token
      App-->>User: Skip F4E update
    end
  end
Loading
sequenceDiagram
  autonumber
  participant Client
  participant Fetch as jade.app.fetch.fetch_from_gitlab
  participant GL as GitLab API
  participant Zip as Archive
  participant FS as Filesystem
  Client->>Fetch: url, path_with_namespace, branch, token
  Fetch->>GL: authenticate(token)
  Fetch->>GL: get project(path_with_namespace)
  Fetch->>GL: download repo archive(branch, zip)
  GL-->>Fetch: zip content
  Fetch->>Zip: _extract_zip(content, tmpdir)
  Zip-->>Fetch: extracted_root_path
  Fetch->>FS: _install_standard_folder_structure(extracted_root, inputs_root, exp_root, ...)
  FS-->>Fetch: success/failure
  Fetch-->>Client: PathLike|bool
Loading
sequenceDiagram
  autonumber
  participant Builder as InputOpenMC
  participant FS as Filesystem
  participant OpenMC as openmc.Settings
  Note over Builder: Initialization
  Builder->>FS: Check for libsource.so
  alt libsource.so exists
    Builder-->>Builder: compiled_source = path
  else
    Builder-->>Builder: compiled_source = None
  end
  Note over Builder: Write
  Builder->>FS: export geometry/materials/tallies/settings
  alt compiled_source available
    Builder->>FS: copy libsource.so to output
    Builder->>OpenMC: settings.source = CompiledSource(libsource.so)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

In burrows of code I hop and fetch,
From IAEA fields to F4E’s stretch.
A tally grows, cumulus bright,
Errors tread softly, weighted right.
OpenMC hums with compiled light—
Agg skies render the silent night.
Carrot-merge: all tests take flight. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (4 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current title “C model doc update” only references documentation changes for the C-Model but does not reflect the significant code, configuration, and test updates introduced in this PR, making it too narrow and potentially misleading. It fails to capture the broad scope of new functionality such as the F4E inputs fetch flow and the cumulative_sum modifier. Please update the title to concisely reflect all major changes, such as documentation enhancements for C-Model tallies, the new F4E inputs fetch functionality, and the cumulative_sum modifier, or consider splitting unrelated code and documentation changes into separate PRs.
Linked Issues Check ⚠️ Warning The pull request adds the detailed enumeration of neutron current, flux, and nuclear heat tallies in the C-Model documentation, satisfying the primary requirement of issue #415, but it does not include the explicit C-Model version in the metadata or alongside the benchmark description as requested. Without a version identifier such as C-Model_R181031, the documentation remains partially compliant. Please add the explicit C-Model version identifier (e.g., C-Model_R181031) in the documentation metadata or alongside the benchmark description to fully comply with issue #415.
Out of Scope Changes Check ⚠️ Warning This PR contains numerous changes unrelated to the C-Model documentation objective of issue #415, including CI workflow updates, secret file ignore rules, new fetch logic for F4E inputs, the cumulative_sum modifier implementation, and broader configuration and test modifications. These out-of-scope changes obscure the intended documentation update. Please remove or isolate unrelated code, configuration, and test updates by moving them to separate PRs so that this PR focuses solely on the C-Model documentation changes required by issue #415.
Docstring Coverage ⚠️ Warning Docstring coverage is 53.57% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch C-model_doc_update

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

@codecov
Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

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

coderabbitai[bot]

This comment was marked as outdated.

@dodu94 dodu94 changed the base branch from master to developing September 30, 2025 10:23
Copy link
Member

@dodu94 dodu94 left a comment

Choose a reason for hiding this comment

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

Thanks!

@dodu94 dodu94 merged commit 8e5fe86 into developing Sep 30, 2025
14 checks passed
@dodu94 dodu94 deleted the C-model_doc_update branch September 30, 2025 10:41
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.

[DOC] - C-Model tally description

3 participants