Make adaptive_levels YAML-usable and reaction-consistent#897
Open
alongd wants to merge 3 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ARC’s adaptive_levels feature to (1) accept a YAML-friendly list-of-entries schema (and correctly round-trip through restart serialization) and (2) enforce reaction-consistent adaptive level selection so barriers aren’t computed using mixed levels of theory across TS/wells.
Changes:
- Replace the legacy tuple-keyed
adaptive_levelsinput format with a YAML-usable list-of-entries schema and update restart serialization accordingly. - Add reaction-wide adaptive level selection via
adaptive_lot_n_heavy, with a new per-speciesthermo_at_own_levelflag controlling whether thermo is computed at the species’ own granular level. - Add/extend unit tests covering restart round-trips and reaction-wide adaptive-level behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/source/advanced.rst | Updates user docs for the new YAML schema and reaction-consistent behavior. |
| arc/species/species.py | Adds thermo_at_own_level and adaptive_lot_n_heavy fields with restart serialization support. |
| arc/species/species_test.py | Adds a unit test verifying the new species fields round-trip through as_dict/from_dict. |
| arc/scheduler.py | Applies reaction-wide adaptive-level logic and uses adaptive_lot_n_heavy in adaptive LOT selection. |
| arc/scheduler_test.py | Adds tests validating reaction-wide consistency, copy creation, and heavy-atom override behavior. |
| arc/main.py | Implements new adaptive_levels list-of-entries parser and updates restart serialization to emit the list form. |
| arc/main_test.py | Updates tests for the new schema, restart round-trip, and legacy-form rejection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3968
to
+3971
| for each such participant whose ``thermo_at_own_level`` is ``True`` (the default), an autonomous relabeled | ||
| copy of the species is created from the outset and used by the reaction (evaluated at the reaction-wide | ||
| level), while the original species is left to compute its own thermochemistry at its own granular level. If | ||
| ``thermo_at_own_level`` is ``False``, the participant itself is evaluated at the reaction-wide level (no copy). |
Comment on lines
+3993
to
+3995
| if not spc.thermo_at_own_level: | ||
| spc.adaptive_lot_n_heavy = reaction_n_heavy | ||
| continue |
Comment on lines
+1297
to
+1301
| if not isinstance(atom_range, (list, tuple)) or len(atom_range) != 2 \ | ||
| or not all(isinstance(a, int) or a in ('inf', float('inf')) for a in atom_range): | ||
| raise InputError(f'The "atom_range" of each adaptive levels entry must be a 2-length list of integers ' | ||
| f'with an optional "inf" upper bound, got {atom_range} in:\n{adaptive_levels}') | ||
| atom_range = (atom_range[0], 'inf' if atom_range[1] in ('inf', float('inf')) else atom_range[1]) |
Comment on lines
+286
to
+288
| By default (the per-species ``thermo_at_own_level`` flag, ``False``) a well that lands on a | ||
| coarser grain than its reaction is evaluated directly at the reaction-wide level, and its | ||
| thermochemistry uses that same (coarser) level. Set ``thermo_at_own_level=True`` on a species |
Tuple keys (1, 6)/(7, 'inf') can't be produced by yaml.safe_load, so adaptive_levels was unusable from an input file. Accept a list of {atom_range, levels} entries, build the tuple-keyed structure internally, and serialize the same form on restart.
Levels were chosen per species from its own heavy-atom count, so a reaction's large TS and its small wells could fall on different grains, mixing levels of theory across the barrier. Key the whole reaction by its (conserved) heavy-atom count, and for any well on a coarser grain make an autonomous relabeled copy that the reaction uses at the reaction-wide level. The new per-species thermo_at_own_level flag (default True) keeps each species' own granular level for thermo; set it False to evaluate the species at the reaction-wide level with no copy.
Reaction wells take the reaction-wide level directly by default (no relabeled copies, no duplicated jobs); set True per species to opt into its own granular level for thermo.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #897 +/- ##
=======================================
Coverage 63.00% 63.00%
=======================================
Files 113 113
Lines 37958 38006 +48
Branches 9956 9967 +11
=======================================
+ Hits 23914 23946 +32
- Misses 11163 11168 +5
- Partials 2881 2892 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes to
adaptive_levels(adaptive level-of-theory), from the roadmap.1. YAML-usable schema
adaptive_levelsrequired Python tuple keys ((1,6),(7,'inf')) thatyaml.safe_loadcan't produce, so it was unusable from an input file. It nowtakes a list of entries, and round-trips through restart:
2. Reaction-consistent levels
Levels were picked per species from its own heavy-atom count, so a reaction's
large TS and its small wells could land on different grains, mixing levels of
theory across the barrier. Each reaction is now keyed by its (conserved)
heavy-atom count, and all of its species are evaluated at that one level.
New per-species thermo_at_own_level flag (default False): set True to
compute a species' thermo at its own granular level — ARC then makes an
autonomous relabeled copy for the reaction so the barrier stays consistent.