Add sp_composite focal-point/CBS single-point protocols#878
Open
Add sp_composite focal-point/CBS single-point protocols#878
Conversation
github-actions
Bot
added
Module: Scheduler
Module: Species
Module: Main
examples
labels
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def test_rejects_equal_cardinals(self): | ||
| with self.assertRaises(InputError): | ||
| helgaker_corr_2pt({3: -1.0, 3: -1.05}) # noqa: F601 — Python collapses; size=1 path |
There was a problem hiding this comment.
Pull request overview
Adds a new sp_composite feature to ARC for HEAT-style / focal-point composite single-point workflows (including CBS extrapolation), with scheduler orchestration + restart support, provenance notebook generation, per-species inherit/override/opt-out semantics, and Arkane explicit-energy rendering when the composite total is injected directly.
Changes:
- Introduces a new
arc/level/package withCompositeProtocol+ term types (delta, CBS extrapolation) + presets, plus backward-compatiblefrom arc.level import Level. - Extends Scheduler to dispatch/track composite sub-jobs, rehydrate restart state, combine energies, and regenerate
<project>/output/sp_composite.ipynb. - Updates Arkane rendering to write
energy = <hartree_float>for composite-finalized species and adds docs/examples/tests for the new YAML forms.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/Composite/per_species_override/input.yml | Example YAML for per-species inherit / null opt-out / explicit protocol. |
| examples/Composite/heat345q_preset/input.yml | Minimal preset-by-name example. |
| examples/Composite/heat345q_partial_override/input.yml | Preset + overrides example. |
| examples/Composite/explicit_fpa/input.yml | Fully explicit recipe example including CBS extrapolation term. |
| examples/Composite/README.md | How-to run examples + explains notebook + units and Arkane behavior. |
| docs/source/advanced.rst | Full user documentation for sp_composite, forms, interactions, restart, notebook, limitations. |
| arc/statmech/arkane_test.py | Tests for Arkane species-file rendering with explicit numeric energy and kJ/mol invariants. |
| arc/statmech/arkane.py | Mako template branch for explicit numeric energy + boundary conversion kJ/mol→Hartree for composite species. |
| arc/species/species_test.py | Tests for 3-state per-species sp_composite model + restart/as_dict round-trips. |
| arc/species/species.py | Adds per-species sp_composite state + protocol storage; persists e_elect_source in restart dict. |
| arc/settings/inputs.py | Clarifies legacy Arkane template is historical and not used by the live renderer. |
| arc/scheduler_test.py | End-to-end tests for composite orchestration, restart rehydration/kick-start, notebook regeneration. |
| arc/scheduler.py | Composite orchestration logic: resolve protocol, queue/dedupe sub-jobs, finalize, notebook reporting, restart rehydration. |
| arc/main_test.py | Project-level YAML plumbing tests: parsing, mutual exclusions, sp_level fallback, AEC/BAC behavior. |
| arc/main.py | Adds sp_composite input handling, mutual exclusions, sp_level fallback, Arkane AEC/BAC routing changes. |
| arc/level/species_state_test.py | Unit tests for INHERIT sentinel + active_composite_for resolution rules. |
| arc/level/species_state.py | Implements INHERIT sentinel and 3-state per-species resolver. |
| arc/level/reporting_test.py | Tests for log formatting and deterministic, executable provenance notebook generation. |
| arc/level/reporting.py | Notebook writer (sp_composite.ipynb) + structured [sp_composite] log event formatter. |
| arc/level/protocol_test.py | Unit tests for composite protocol model and validation rules. |
| arc/level/protocol.py | Implements CompositeProtocol + Term types + CBS extrapolation + safe user formula evaluation integration. |
| arc/level/presets_test.py | Tests preset registry, expansion, override validation, and round-trips. |
| arc/level/presets.yml | Ships built-in presets (HEAT-345, HEAT-345Q, FPA-min) with references/DOIs. |
| arc/level/presets.py | Loads presets.yml and applies validated deep-merge overrides. |
| arc/level/level_test.py | Regression tests for Level args parsing (string/iterable). |
| arc/level/level.py | Fixes Level.lower() args handling bug for string args. |
| arc/level/legacy_imports_test.py | Guards backward-compatible imports from arc.level. |
| arc/level/examples_test.py | Ensures shipped examples parse and build valid CompositeProtocols. |
| arc/level/cbs_test.py | Tests cardinal inference, built-in formulas, and safe AST evaluation. |
| arc/level/cbs.py | Implements cardinal inference, built-in CBS formulas, and safe AST evaluator. |
| arc/level/init.py | Re-exports legacy symbols + exposes INHERIT/state resolver helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1455
to
+1458
| elif parser.parse_e_elect(path) is None: | ||
| reason = "parse_e_elect returned None" | ||
| else: | ||
| continue |
There was a problem hiding this comment.
parser.parse_e_elect(path) can raise exceptions (e.g., adapter parsing errors). In restart rehydration this would crash ARC instead of simply invalidating/re-queuing the sub-job. Wrap the parse_e_elect call in a try/except and treat any exception as unparseable (invalidate with the exception message as the reason).
Suggested change
| elif parser.parse_e_elect(path) is None: | |
| reason = "parse_e_elect returned None" | |
| else: | |
| continue | |
| else: | |
| try: | |
| e_elect = parser.parse_e_elect(path) | |
| except Exception as e: | |
| reason = f"parse_e_elect raised: {e}" | |
| else: | |
| if e_elect is None: | |
| reason = "parse_e_elect returned None" | |
| else: | |
| continue |
Comment on lines
+1563
to
+1571
| missing: List[str] = [] | ||
| for _term_label, sub_label, _level in protocol.iter_required_jobs(): | ||
| path = completed.get(sub_label) | ||
| if not path: | ||
| missing.append(sub_label) | ||
| continue | ||
| value = parser.parse_e_elect(path) | ||
| if value is None: | ||
| missing.append(sub_label) |
There was a problem hiding this comment.
Composite finalization also calls parser.parse_e_elect(path) without guarding against exceptions. A single malformed/partial output could raise and abort the entire scheduler loop. Catch exceptions here as well and include the exception in the missing/warning payload so the species is not stuck silently.
Suggested change
| missing: List[str] = [] | |
| for _term_label, sub_label, _level in protocol.iter_required_jobs(): | |
| path = completed.get(sub_label) | |
| if not path: | |
| missing.append(sub_label) | |
| continue | |
| value = parser.parse_e_elect(path) | |
| if value is None: | |
| missing.append(sub_label) | |
| missing: List[Dict[str, str]] = [] | |
| for _term_label, sub_label, _level in protocol.iter_required_jobs(): | |
| path = completed.get(sub_label) | |
| if not path: | |
| missing.append({ | |
| "sub_label": sub_label, | |
| "reason": "missing path", | |
| }) | |
| continue | |
| try: | |
| value = parser.parse_e_elect(path) | |
| except Exception as e: | |
| missing.append({ | |
| "sub_label": sub_label, | |
| "path": path, | |
| "reason": "parse exception", | |
| "exception": repr(e), | |
| }) | |
| continue | |
| if value is None: | |
| missing.append({ | |
| "sub_label": sub_label, | |
| "path": path, | |
| "reason": "unparseable", | |
| }) |
Comment on lines
+736
to
+737
| # Arkane's file-format expects Hartree; our conversion. | ||
| self.assertIn(f"energy = {expected_hartree}", content) |
There was a problem hiding this comment.
This assertion is brittle because it depends on Python's default float-to-string rendering matching exactly what Mako emits (precision/scientific-notation differences can cause intermittent failures). Prefer asserting via regex extraction + numeric comparison (similar to the later test), or format the expected value with the same precision used in the template.
Suggested change
| # Arkane's file-format expects Hartree; our conversion. | |
| self.assertIn(f"energy = {expected_hartree}", content) | |
| # Arkane's file-format expects Hartree; verify the rendered numeric value | |
| # without depending on exact float string formatting. | |
| match = re.search(r"^energy = ([-+]?\d*\.?\d+(?:[eE][-+]?\d+)?)$", content, re.MULTILINE) | |
| self.assertIsNotNone(match) | |
| self.assertAlmostEqual(float(match.group(1)), expected_hartree) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #878 +/- ##
==========================================
+ Coverage 60.46% 61.15% +0.68%
==========================================
Files 102 108 +6
Lines 31096 31900 +804
Branches 8103 8268 +165
==========================================
+ Hits 18803 19509 +706
- Misses 9953 9993 +40
- Partials 2340 2398 +58
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:
|
alongd
force-pushed
the
sp_composite
branch
2 times, most recently
from
5c0d74f to
7fe1f85
Compare
Move arc/level.py -> arc/level/level.py (and its test) so the package can host additional level-of-theory machinery without bloating one module. arc/level/__init__.py re-exports the legacy surface so every ``from arc.level import Level`` caller keeps working unchanged. legacy_imports_test.py guards that contract. No behavior change.
Scheduler-agnostic foundation for composite single-point protocols (HEAT-style focal-point analysis + CBS extrapolation): - protocol.py: Term hierarchy (SinglePoint/Delta/CBSExtrapolation) + CompositeProtocol. Accepts preset names, preset+overrides, or explicit recipes. Validates unique term labels and sub_labels, formula arity, and rejects components != "total" until per-component parsing exists. - cbs.py: cardinal_from_basis for cc-pV*Z / def2 families; built-in formulas (helgaker_corr_2pt, helgaker_hf_2pt, martin_3pt) with citations; whitelist AST evaluator for user formulas (no eval). - presets.py + presets.yml: HEAT-345, HEAT-345Q, FPA-min with DOIs. Overrides deep-merge nested Level dicts and reject unknown targets/fields. - species_state.py: INHERIT sentinel + active_composite_for helper for the 3-state per-species model (inherit / opt_out / explicit). - reporting.py: SpeciesSection dataclass + write_composite_notebook emitting a project-level unexecuted .ipynb. Each section is self-contained and re-parses QM outputs on Run-All so the user independently verifies the computed energy. format_log_event for structured [sp_composite] log lines. Tests: unit coverage per module plus an end-to-end nbclient test that executes the generated notebook against fixture Gaussian outputs.
Legacy behavior byte-for-byte unchanged when sp_composite is absent. main.py: parse sp_composite YAML into a CompositeProtocol; raise InputError when combined with composite_method or adaptive_levels; derive sp_level from protocol.base.level when omitted. Route Arkane AEC through base.level; skip BAC with one warning. species.py: 3-state ARCSpecies.sp_composite with INHERIT sentinel so "key absent" vs "key: null" are distinguishable; e_elect_source provenance flag. Both round-trip through as_dict/from_dict. scheduler.py: run_sp_job uses protocol.base.level for active-composite species; opt==sp shortcut gated off. post_sp_actions branches into a composite flow that Level-matches completions to pending sub_labels (de-duplicating shared Levels), spawns remaining sub-jobs, and finalizes by parsing every sub-job via parse_e_elect, calling protocol.evaluate, and writing species.e_elect (kJ/mol) plus e_elect_source='sp_composite'. Regenerates the project-level notebook from the cumulative SpeciesSection list. On init, rehydrates from the persistent output dict, validates recorded paths (missing/unparseable entries are pushed back to pending with a warning), and kick-starts any remaining sub-jobs. Structured [sp_composite] log events at every transition. arkane.py: generate_species_file renders a bare numeric ``energy = <Hartree>`` (converted once via E_h_kJmol) when e_elect_source == 'sp_composite'. Raises ValueError if the source is set but e_elect is None. Unit invariant: parse_e_elect returns kJ/mol; species.e_elect stays kJ/mol; Hartree appears only at log display and Arkane render. Tests cover: top-level parsing + mutex; sp_level fallback and preservation; AEC-to-base + BAC-skipped warning; ARCSpecies 3-state round-trips; Scheduler orchestration end-to-end with fixture Gaussian outputs incl. shared-Level de-dup, TS, per-species override/opt-out; log-event capture; restart reuse + corruption recovery + kick-start; rehydrated species reappear in regenerated notebook; Arkane composite vs legacy rendering; dh_rxn consumes composite e_elect in kJ/mol.
docs/source/advanced.rst grows a "Composite single-point protocols (sp_composite)" subsection covering all four YAML forms (preset / preset+override / explicit recipe with CBS / per-species override), interactions with sp_level, composite_method, adaptive_levels, and conformer_sp_level, AEC routing + BAC-skipped-with-warning policy, restart behavior, the provenance notebook + Run-All workflow, units, and limitations. References with DOIs for HEAT, Helgaker/Halkier CBS, Martin 3-pt, and Dunning basis-set families. examples/Composite/ ships a README and four runnable inputs: * heat345q_preset — preset by name * heat345q_partial_override — preset with overrides * explicit_fpa — explicit recipe incl. CBS term * per_species_override — mixed inherit/null/explicit The README flags HEAT-style post-(T) examples as illustrative and calls out explicit_fpa as the affordable demo. Tests: arc/level/examples_test.py YAML-parses every shipped example and builds every sp_composite block via CompositeProtocol.from_user_input, and asserts that all four forms appear so docs and examples stay in sync.
Two compounding bugs surfaced on a real HEAT-345Q production run on
HOCHO/CH2O2: δ_CV silently evaluated to 0, and δ_rel was implausibly
large (~25 kJ/mol).
Bug A — preset emission. The HEAT-345Q δ_CV and δ_rel terms shipped
with malformed args.keyword payloads — the molpro adapter writes the
*value* of each args.keyword entry but not the key, so
``{fc: '0'}`` rendered as a bare ``0`` line ("Unknown command or
directive") and ``{rel: dkh2}`` rendered as ``dkh2`` (silently ignored).
Rewrite both terms with canonical molpro syntax:
* δ_CV high (all-electron): ``args.keyword.core: 'core,0,0,0,0,0,0,0,0;'``
* δ_rel high (DKH2): ``args.keyword.dkho: 'SET,DKHO=2;'`` — the
Molpro manual explicitly recommends ``DKHO`` over the legacy
``DKROLL`` (https://www.molpro.net/manual/doku.php?id=relativistic_corrections).
Both directives land in the ``${keywords}`` slot between ``basis=``
and ``int;`` so integrals are evaluated with the right Hamiltonian.
Verified by rendering the molpro input via the adapter against a
HOCHO fixture xyz and inspecting the generated ``input.in`` for both
δ_CV/δ_rel high and low legs. Add HEAT-345_noC and HEAT-345Q_noC
variants that drop δ_CV (reference string spells out "OMITTED" so
users cite honestly) for ESSes without a clean all-electron syntax.
Bug B — Level structural equality. Even after the preset fix,
``Level.__eq__`` delegated to ``str(self)`` which dropped ``args``
whenever any sibling bucket was empty (``block: {}``). Two protocols
differing only in args.keyword (the all-electron core directive vs
default frozen-core) compared equal, so the composite-spawn
deduplication collapsed the high+low δ_CV legs into one molpro job
and computed δ_CV = E[same] − E[same] = 0. Replace ``__eq__`` with
attribute-by-attribute structural comparison; fix the matching
``all(values)`` skip in ``__str__`` and ``as_dict`` so args content
participates in stringify/serialize whenever any bucket has content;
explicitly mark ``__hash__ = None`` (Level was already unhashable in
practice — no set/dict-key usage in the repo).
Tests: 6 new in level_test.py (eq distinguishes args.keyword,
identical levels remain equal, as_dict round-trips args, str shows
keyword args even with empty block, __hash__ is None); 4 new in
presets_test.py (HEAT-345_noC / HEAT-345Q_noC presence, omit δ_CV,
reference advertises the omission, δ_CV legs of the original HEAT
variants are no longer mistakenly equal); update one snapshot in
main_test.py::TestARC::test_as_dict to reflect args being included.
Docs: presets.yml header explains the ESS-specific args.keyword
contract and links the Molpro manual; advanced.rst lists the new
``_noC`` variants and the SET,DKHO=2 / core,0,... directives used.
Tests: pytest arc/level/ arc/main_test.py arc/species/species_test.py
arc/scheduler_test.py arc/statmech/arkane_test.py -q -> 360 passed.
Bug C (generic ESS trsh swaps the basis to ``cc-pVDZ`` for composite
sub-jobs, and a successful sister retry doesn't short-circuit the
original failed job's later trsh chain) is a known follow-up on the
same PR — wastes compute but produces no wrong results.
| """Custom __eq__ without a matching __hash__ ⇒ unhashable. | ||
| Locks the contract; nothing in the codebase puts Level into a set/dict-key.""" | ||
| with self.assertRaises(TypeError): | ||
| hash(Level(method='hf', basis='cc-pVTZ')) |
set_job_args fired a warning saying it was discarding ``level.args`` on every first-run job whose level carried args. The trace shows the opposite: ``Scheduler.run_job`` already merges ``level.args`` into the local ``args`` dict (``args.update(level_of_theory.args)``) before calling job_factory, so by the time set_job_args sees ``args`` it's a superset of ``level.args``. Nothing was actually being ignored — the warning lied and added noise to every composite sub-job log line. Simplify the function: keep the legacy convenience fallback (empty ``args`` + level with ``level.args`` → use ``level.args``), guarantee the keyword/block/trsh buckets, and drop the spurious warning. Also drops the now-unused ``pformat`` import. Tests: existing test_set_job_args still passes; 2 new regression tests lock the no-warning contract on a typical first-run path with ``args.keyword.core`` content, and the args-None-falls-back-to-level behavior. Full sweep: pytest arc/level/ arc/main_test.py arc/species/species_test.py arc/scheduler_test.py arc/statmech/arkane_test.py arc/job/adapters/common_test.py -q -> 374 passed.
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.
Summary
Add
sp_composite: composite single-point energy protocols for ARC.This adds HEAT-style / focal-point composite SP workflows and CBS extrapolation, with:
Legacy
composite_methodremains unchanged.User-facing behavior
New YAML key:
Supported forms:
overridesbase+correctionssp_composite: nullNotes:
composite_methodadaptive_levelssp_levelis omitted, ARC usessp_composite.base.levelconformer_sp_levelis unchangedcomponents: totalonlyImplementation
Levelinto the newarc/level/package while preservingfrom arc.level import LevelCompositeProtocol,SinglePointTerm,DeltaTerm, andCBSExtrapolationTermpresets.yml