fix(statmech): declare reaction species in the Arkane kinetics input regardless of compute_thermo - #1022
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1022 +/- ##
==========================================
+ Coverage 64.61% 64.63% +0.01%
==========================================
Files 119 119
Lines 39830 39831 +1
Branches 10313 10312 -1
==========================================
+ Hits 25736 25743 +7
+ Misses 11116 11104 -12
- Partials 2978 2984 +6
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:
|
There was a problem hiding this comment.
Pull request overview
Fixes ARC-generated Arkane kinetics inputs that referenced reactant/product labels in reaction(...) blocks without declaring the corresponding species(...) entries when ARCSpecies.compute_thermo=False, which could cause Arkane to fail early with a KeyError and produce no kinetics output.
Changes:
- Ensure any species named by reactions is included in the Arkane input
species_listregardless ofcompute_thermo(while preserving the existing gate for non-reaction species). - Add a unit test covering kinetics-input rendering when reactants/products have
compute_thermo=False.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| arc/statmech/arkane.py | Expands the species-declaration gate so kinetics renders always declare reaction participants, preventing Arkane KeyError on undeclared species labels. |
| arc/statmech/arkane_test.py | Adds regression test asserting reaction species are declared in kinetics inputs even when compute_thermo=False. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
442682d to
45ca6e1
Compare
…tion species in the Arkane kinetics input regardless of compute_thermo
Should be fixed in #1001 |
…rdless of compute_thermo render_arkane_input_template gated species(...) declarations on 'e0_only or spc.compute_thermo'. A kinetics render (e0_only=False) whose caller sets compute_thermo=False on reactants/products then emitted a reaction(...) that named species it never declared, so Arkane raised KeyError on the reactant label and produced no rate coefficient, despite all QM jobs having converged. Also declare any species named by a reaction in the render's reaction list. Thermo inputs (no reactions) are unchanged; compute_thermo still governs whether a thermo job runs, not whether a species may be declared in an input that references it.
45ca6e1 to
4fbafa6
Compare
|
You're right — checked #1001 and it fixes exactly this: |
What this fixes
An Arkane kinetics input generated by ARC can reference species it never declares, so Arkane dies before computing anything:
This was found on a run where everything else went right: two species and a transition state all optimised and frequency-checked on a cluster, and then no rate coefficient at all.
Cause
ArkaneAdapter.render_arkane_input_templatebuildsspecies_listunderif e0_only or spc.compute_thermo:. A caller that setscompute_thermo=False— as an orchestrator does when it wants kinetics but does not want a full thermodynamics job queued — gets:e0_only=True) → gate passes → all species declared;e0_only=False) → gate fails → none declared, whilereaction(...)still names them by label.generate_species_fileswrites the per-species statmech files either way, so ARC writesstatmech/kinetics/species/<label>.pyto disk and then writes a main input that references that species without declaring it.compute_thermoshould govern whether a thermo job runs — not whether a species may be declared in an input that names it.The existing
test_generate_arkane_inputnever caught this becauseARCSpecies.compute_thermodefaults tonot is_ts, i.e.Truefor ordinary species; the defect only surfaces when a caller sets it toFalseexplicitly.The change
Collect the labels named by any reaction in the render's reaction list, and let a species through the gate if it is one of them:
Deliberately narrow rather than dropping the gate: dropping it would push species into thermo inputs that exclude them on purpose. For a thermo render
self.reactionsisNone, so the new set is empty and the gate is unchanged — thermo inputs are unaffected.Testing
compute_thermo=Falsenow renders as aspecies('R', ...)line in a kinetics input.reaction(reactants=['R'], products=['P'])above zerospecies(lines.arc/statmech/: 50 passed.species(...)declarations this fix emits, Arkane exits 0 and writes the rate coefficient it previously could not produce:maininarkane/, so treat them as a plumbing result, not a physical one.)Note for anyone running these tests:
arc/statmech/needs-n0. Under the default xdist configtest_generate_arkane_inputflakes, because sibling workers share the on-diskarc/testing/arkane_input_tests_deletedirectory. Pre-existing and unrelated to this change.Separately: an unrelated bug found on the same run
arc/job/ssh.py:42-43unpackedself.connect(), which returnsNone. Not fixed here, and no longer needs to be: @calvinp0 pointed out it is already fixed in #1001 (self._sftp, self._ssh = self.connect()→self.connect(), with a regression test). Tracked there.