VULCAN online chemical kinetics calculations#596
VULCAN online chemical kinetics calculations#596ipanagiotou02 wants to merge 28 commits intomainfrom
Conversation
…t a VULCAN calculation to avoid repeating
There was a problem hiding this comment.
The tests are failing because the run_chemistry function is returning None rather than the Pandas DataFrame of mixing ratios. See comment below about avoiding test regression for offline chemistry, and adding new test for online chemistry.
Also the code style check is failing, just because of some whitespace discrepancies. Also should be easy to fix.
Are the offline/online/manual chemistry 'modes' described in the documentation anywhere? The docs should also briefly explain these differences and their reasons for other users.
In principle these changes look good. Should be easy to fix, and would then be ready to be merged into main :)
There was a problem hiding this comment.
Pull request overview
This PR implements online chemical kinetics calculations by integrating VULCAN to run at every snapshot during a PROTEUS simulation, rather than only as a post-processing step. This enables tracking of atmospheric chemistry evolution throughout the simulation, though at a significant computational cost.
Changes:
- Added online chemistry mode to run VULCAN at every snapshot alongside offline and manual modes
- Implemented snapshot tracking to avoid redundant VULCAN calculations
- Created timestamped output files for each VULCAN run in online mode
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 18 comments.
| File | Description |
|---|---|
| src/proteus/proteus.py | Added snapshot detection logic and VULCAN invocation at each snapshot in the main simulation loop |
| src/proteus/atmos_chem/wrapper.py | Extended run_chemistry to handle online, offline, and manual modes with appropriate branching logic |
| src/proteus/atmos_chem/vulcan.py | Added run_vulcan_online function with timestamped file outputs for per-snapshot chemistry calculations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…atmos_chem module used Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@ipanagiotou02 thanks for making these changes after my review :) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #596 +/- ##
==========================================
+ Coverage 60.77% 63.50% +2.72%
==========================================
Files 98 98
Lines 9388 9420 +32
Branches 1263 1274 +11
==========================================
+ Hits 5706 5982 +276
+ Misses 3383 3136 -247
- Partials 299 302 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ipanagiotou02
left a comment
There was a problem hiding this comment.
Copilot Feedback
…641) * Fix broken control flow in atmos_chem wrapper and add tests The if/elif chain in wrapper.py was structurally broken: the offline branch only logged a message, then fell through to a new if/module chain that always called run_vulcan_offline, making online mode unreachable. Replace with clean dispatch on the 'when' config value. - Fix wrapper.py: branch on when={manually,offline,online} correctly - Remove duplicate run_vulcan_offline import - Add inline comments explaining dispatch logic - Update existing tests to set explicit when='offline' - Add wrapper branch tests for all when modes + invalid value - Add test_vulcan.py with 10 unit tests covering run_vulcan_online (AGNI guard, per-snapshot filenames, one-time network compilation, missing result detection, network selection, exist_ok directory) - wrapper.py coverage: 47% -> 100%, vulcan.py coverage: 0% -> 62% Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: expand VULCAN chemistry modes documentation in usage guide Replace the brief 'offline chemistry' section with a comprehensive guide covering all three atmos_chem.when modes (manually, offline, online), their behaviour, output files, and configuration options. Update the output tree to show per-snapshot files from online mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix read_result to support per-snapshot filenames for online mode Online VULCAN writes vulcan_{year}.csv but read_result always looked for vulcan.csv. Add optional filename parameter to read_result and pass the per-snapshot filename from wrapper when when='online'. * Merge run_vulcan_offline and run_vulcan_online into single run_vulcan The two functions shared ~300 lines of identical VULCAN config code. Merge them into run_vulcan(dirs, config, hf_row, *, online=False) with the 4 actual differences handled by the online flag: - Directory setup: offline wipes+recreates, online uses exist_ok - Filenames: offline uses fixed names, online uses per-snapshot - Network compilation: offline always compiles, online only once - Path construction: unified to os.path.join throughout Addresses review comment by nichollsh on PR #641. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
All changes were made on the sub-branch.
Description
(This is a draft PR; copied comment from Issue #6)
In the branch
ip_vulcan, the online calculation has been implemented such that it calls VULCAN at every loop (as of now). It creates avulcan.csvfile at every snapshot corresponding to the.ncfile of the same time. For example, if the file66097_atm.ncis created in the data directory,vulcan_66097.csvis created in the offchem directory. This is still a post-processing approach, but it occurs at every snapshot instead of just at the end.From my project, I knew that a simulation with
orbit.semimajoraxis = 0.0316andoutgas.fO2_shift_IW = -4.0converges relatively quickly (required 29 minutes), so I used it as an initial test. The results are shown below:A few issues/remarks to make are the following:
Time: VULCAN's runtime can range from 10 minutes to more than 1 hour, depending on the number of steps it needs to converge. In most cases, convergence is difficult to achieve, and it takes all 30,000 steps to run. This adds a lot of time for a simulation run (the above case went from 29 minutes to 5.29 hours).
Memory: due to the above, VULCAN greatly increases the size of the proteus_00.log file (from 650 KB to 22 MB). Additionally, each additional .csv file adds 650 KB, and each additional .pkl file adds 5 MB. The example simulation produced 29 of these files times 2 for each, increasing the total simulation memory by 160 MB. For the all_options case, a run with 6 GB allowed the simulation to run for 63 loops and produce 57 vulcan.csv files/snapshots, but without converging. Here is the result up to the point it ran:
Any feedback that you have is appreciated!
Checklist
Relevant people
@timlichtenberg @nichollsh @shami-EEG