Skip to content

Conversation

@jan-janssen
Copy link
Member

@jan-janssen jan-janssen commented Oct 3, 2025

Summary by CodeRabbit

  • New Features

    • Added progress tracking to the Quantum ESPRESSO energy–volume evaluation flow.
  • Refactor

    • Notebooks now use public workflow APIs and adopt a task-oriented data flow for energy–volume generation and analysis.
  • Chores

    • Constrained per-task threading for more stable parallel runs.
    • CI notebook step now lists Flux resources before running notebooks.
    • Added progress utility support for evaluations.
  • Documentation

    • Updated notebook text and cleaned execution metadata for clarity and reproducibility.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Warning

Rate limit exceeded

@jan-janssen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 17 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between b28a413 and 240165b.

📒 Files selected for processing (1)
  • notebooks/5-1-gpaw.ipynb (9 hunks)

Walkthrough

Replaces internal EV-curve helpers in two notebooks with public wrappers from atomistics.workflows, renames structure_dicttask_dict, updates task generation/analysis calls, constrains GPAW threading/MPI environment, and adds a Flux resource-list step to the notebooks CI pipeline.

Changes

Cohort / File(s) Summary of Changes
EV workflow migration — GPAW notebook
notebooks/5-1-gpaw.ipynb
Replaced imports from evcurve.helper with public atomistics.workflows functions (get_tasks_for_energy_volume_curve, analyse_results_for_energy_volume_curve); structure_dicttask_dict wiring; task construction now uses task_dict["calc_energy"]; evaluate_with_gpaw sets threading/MPI env vars; minor narrative/metadata updates.
EV workflow migration — Quantum ESPRESSO notebook
notebooks/5-2-quantum-espresso.ipynb
Same import and API migration: use get_tasks_for_energy_volume_curve / analyse_results_for_energy_volume_curve; structure_dicttask_dict and task_loop_dict built from task_dict["calc_energy"]; executor calls updated to pass task_dict; added tqdm for progress; cell execution_count/metadata tweaks.
Public API surface
atomistics.workflows (exports)
Added public wrappers: get_tasks_for_energy_volume_curve and analyse_results_for_energy_volume_curve; call-site signatures adjusted (axes param added to task generation; task_dict usage replaces structure_dict).
CI workflow tweak
.github/workflows/pipeline.yml
In notebooks_integration job, Notebooks environment step now runs flux start flux resource list prior to papermill notebook actions (lists Flux resources before notebook execution).

Sequence Diagram(s)

sequenceDiagram
  participant NB as Notebook
  participant WF as atomistics.workflows
  participant EX as Executor (GPAW / QE)
  participant OUT as Results

  NB->>WF: get_tasks_for_energy_volume_curve(structure, num_points, vol_range, axes?)
  WF-->>NB: task_dict
  NB->>EX: submit tasks from task_dict["calc_energy"] (GPAW sets OMP_NUM_THREADS / MPI env)
  EX-->>OUT: output_dict (energies, volumes, metadata)
  NB->>WF: analyse_results_for_energy_volume_curve(output_dict, task_dict, fit_type, fit_order)
  WF-->>NB: E–V fit results
  note right of NB: QE notebook may show progress via tqdm
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

I hop through volumes, light and free,
Tasks bundled tidy — task_dict for me.
Old helpers shelved, new wrappers call,
Threads kept gentle, results answer the hall.
A rabbit stamps — the curves fit all 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “Fix integration notebook” is too generic and does not clearly convey the primary changes in this pull request, which include migrating energy‐volume workflows to public atomistics.workflows APIs, updating notebook code accordingly, and adding a Flux command to the CI pipeline. It refers only broadly to a notebook fix without specifying the actual scope of API and workflow updates across multiple notebooks and the CI configuration. This lack of specificity makes it difficult for reviewers to understand the main purpose at a glance. Please update the title to more precisely reflect the key changes, such as “Migrate EV‐curve notebooks to public atomistics.workflows API and add Flux command to CI,” so that it clearly summarizes the scope of the updates.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

@codecov
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.16%. Comparing base (c9de13a) to head (240165b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #832   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files          34       34           
  Lines        1686     1686           
=======================================
  Hits         1655     1655           
  Misses         31       31           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@jan-janssen jan-janssen closed this Oct 4, 2025
@jan-janssen jan-janssen deleted the notebooks branch October 4, 2025 08:37
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