fix(pdep): resolve the explorer seed to the round's own species label by alongd · Pull Request #199 · ReactionMechanismGenerator/T3

alongd · 2026-08-23T19:42:14Z

Stacked on #198, which is stacked on #197. Base is i018-modelchem, so this diff shows only its own change; GitHub retargets as each parent merges. Review #197 → #198 → this.

What this fixes

Third defect on T3's round-0 → round-1 handoff. With #197 and #198 in place, re-exploring a hybrid network now fails at:

ValueError: Seed species label '[O]C=O' is not defined as a species() block anywhere in the source.
Known species labels are ['Ar', 'O=C=O(5)', 'O=[C]O(8)', '[H](6)', '[O]C=O(1)'].

The loop configures its explorer seed once, from pes.source, and _build_explorer_config passes it verbatim in every round. [O]C=O is a literal species() label in the source network, so round 0 works. From round 1 the loop re-explores a hybrid in which Arkane has relabelled that species [O]C=O(1), and the seed matches nothing.

The change

_resolve_seed_labels, called immediately before _validate_seed_species, resolves the configured seed to the label the round's own network uses. Matching is by canonical structure (SMILES + multiplicity — the same identity pes_rounds already uses for channel keying), reading the seed as a SMILES and comparing against each species' structure.

Per-round structural resolution rather than carrying forward round 0's resolved label: carry-forward cannot cover the case where the loop is started on a hybrid (there is no prior round to carry from), needs cross-round state, and is not robust to relabelling between any two rounds.

A literal label that already matches is passed through untouched, so round-0 behaviour is byte-for-byte unchanged, and _validate_seed_species is not loosened — its label-equality test is correct, because Arkane resolves source against species_dict by label. The resolution happens before it, not inside it.

Behaviour at the edges:

Zero matches (no structural match, or the seed is not a parseable SMILES) — the seed passes through unchanged and _validate_seed_species raises its own diagnostic "not defined … known labels are […]". Fail-closed; the resolver never invents a label.
Multiple matches (two species with identical canonical structure) — refused, naming the candidates. A source configuration must name exactly one physical species.

Testing

tests/data/pdep_hybrid/cho2_round0/network0_reduced.py is the real round-0 hybrid from a cluster run, committed verbatim.

Real-artifact tests drive it through _resolve_seed_labels ([O]C=O → [O]C=O(1)) and through write_arkane_explorer_input_file.
Also adds the real-data test that feat(explorer): accept the modelChemistry LevelOfTheory call form in the source guard #198 lacked: its modelChemistry acceptance is now pinned against the real file rather than only synthetic inputs.
Mutation: neutering the resolver to a pass-through turns three tests red — the real-data resolution, the full-seam past-seed-validation, and the multi-match refusal. Restored, green.
tests/test_pdep: 1922 passed, 1 xfailed, 134 s.

Known next defect, pinned rather than chased

The run now advances two guards further and stops at _validate_generated_statements (input_file.py:1607) on the generated side, rejecting the same modelChemistry = LevelOfTheory(...) line that #198 taught the source-side guard to accept. The two guards are symmetric and #198 only reached one of them; the line is copied verbatim source → generated and is already validated on the way in.

That is a separate change, tracked separately. It is pinned here by an xfail(strict=True) test, so it fails loudly the moment the fix lands and cannot be forgotten.

Copilot

Pull request overview

This PR improves robustness of T3’s PDep explorer round-to-round handoff when re-exploring hybrid networks by resolving configured explorer seed labels to the current round’s species labels via structural identity (canonical SMILES + multiplicity), preventing failures caused by Arkane relabeling (e.g. "[O]C=O" → "[O]C=O(1)").

Changes:

Add per-round seed-label resolution (_resolve_seed_labels) in write_arkane_explorer_input_file, performed before _validate_seed_species, and surface any remap notes as warnings.
Expand fixtures + tests using real hybrid artifacts to pin the round-0 → round-1 seam behavior (including seed resolution and modelChemistry acceptance).
Allow the specific Arkane-required positional form transitionState(label, path) in the pdep parser to handle real hybrid outputs.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`t3/pdep/explorer/input_file.py`	Adds structural seed-label resolution and extends source-statement validation for `modelChemistry` call forms.
`t3/pdep/parser.py`	Adds a narrow exception to accept positional `transitionState(label, path)` calls.
`tests/test_pdep/test_explorer_input_file.py`	Adds real-artifact tests for seed resolution and modelChemistry acceptance; pins a known next defect via strict xfail.
`tests/test_pdep/test_parser.py`	Adds real-artifact tests to ensure positional `transitionState` parsing works and remains narrowly scoped.
`tests/data/pdep_hybrid/positional_ts/README.md`	Documents provenance and rationale for the vendored positional-TS hybrid fixture.
`tests/data/pdep_hybrid/positional_ts/network0_reduced.py`	Adds a real hybrid network fixture containing `transitionState('TS2', 'qm/TS2.py')`.
`tests/data/pdep_hybrid/cho2_round0/network0_reduced.py`	Adds a real hybrid network fixture used to validate seed relabeling behavior.
`.gitignore`	Ignores `docs/contracts/` directory.

Suppressed comments (1)

t3/pdep/explorer/input_file.py:667

_species_structure_identity only catches (ValueError, KeyError) around Molecule parsing. To keep seed resolution fail-closed on unexpected adjacencyList/SMILES parsing failures (and consistent with other Molecule parsing helpers), this should catch Exception so a malformed/edge-case structure does not crash the whole write path.

    except (ValueError, KeyError):

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov-commenter · 2026-08-25T10:01:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.93%. Comparing base (3da8c00) to head (c758604).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #199      +/-   ##
==========================================
- Coverage   82.73%   81.93%   -0.80%     
==========================================
  Files          72       74       +2     
  Lines       11733    12019     +286     
  Branches     2530     2586      +56     
==========================================
+ Hits         9707     9848     +141     
- Misses       1435     1556     +121     
- Partials      591      615      +24

Flag	Coverage Δ
unittests	`81.93% <ø> (-0.80%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

The PES loop configures its explorer seed once, from pes.source (a SMILES label like '[O]C=O') that is a literal species() label in the source network, and passes it verbatim every round. From round 1 on the loop re-explores a HYBRID network whose species Arkane relabelled with network indices ('[O]C=O(1)'), so the configured label is no longer a literal label and _validate_seed_species refuses it (by-label, like Arkane's own source resolution) -- the run dies at round 1. Resolve the seed to the label THIS network uses BEFORE it reaches the validator, by canonical molecular structure (SMILES|multiplicity, the same identity t3.pdep.pes_rounds uses for channel keying), never by loosening the validator. A literal match is kept untouched (round-0 behaviour is unchanged); a seed that matches no species by structure, or is not a parseable SMILES, is passed through so the validator still raises its clear error; a seed matching more than one species is refused as ambiguous. Tests drive the real round-0 hybrid of the r001_m1-cho2-pilot run (copied into tests/data), since every defect on this handoff was invisible to synthetic fixtures. One xfail(strict) test drives the same real hybrid through the full writer and pins a further defect carried forward from the branch below: _validate_generated_statements rejects the 'modelChemistry = LevelOfTheory(...)' line that _validate_source_statements accepts, so the real hybrid does not yet survive the full writer. That generated-side guard is left for its own ticket.

…line the inbound guard admits The inbound source guard learned to accept `modelChemistry = LevelOfTheory(...)`, but the outbound self-check was never taught the same. Because that line is spliced verbatim from source to generated file, it passed on the way in and was refused on the way out, so the explorer input file was never written. Share one accept-test (`_is_model_chemistry_call_assignment`) between both guards so they cannot drift apart again, and correct the outbound error message, which asserted "no validated source statement can become" this line -- now false. Adds focused outbound-acceptance/refusal unit tests, turns the pinned xfail(strict) real-hybrid test into an ordinary passing test, and updates the seed-barrier test that pinned the old stop-point.

…s directory The hybrid writer emits transitionState('TS2', 'qm/TS2.py') with the qm/ tree vendored beside the hybrid -- a path relative to the hybrid, deliberately portable. But write_arkane_explorer_input_file splices that block verbatim into a generated Arkane explorer input in a DIFFERENT directory with no qm/ sibling, so Arkane resolves the raw 'qm/TS2.py' against its own working directory and dies with FileNotFoundError at arkane/statmech.py load time. Rewrite each spliced species()/transitionState() external file path (Arkane's <name>(label, path) form) from relative to absolute, anchored at the SOURCE file's own directory, so it resolves wherever the generated file lands and whatever cwd Arkane runs in. An absolute path is passed through Arkane's os.path.join rebase unchanged; the inner Log('logs/...') references then rebase (statmech.py:319) relative to the now-correct species-file directory and resolve too. The hybrid's relative convention is untouched -- only the spliced copy is edited -- and a string literal stays a string literal, so both guards in the module still accept it. The rewrite is recorded in ExplorerInputSummary.stat_mech_paths_absolutized. Regression test drives the real committed round-0 hybrid through the writer into an unrelated directory and asserts the emitted path resolves to a file that exists (os.path.isfile), with the real qm/TS2.py vendored beside the fixture.

…k invariant The hybrid splice put a full-conformer QM transition state (translation + rotation + vibration) onto vibration-only estimated wells, so TST's Q(TS)/Q(reactant) never cancelled the TS's translational/rotational partition functions (~1e14). k(E) exploded and the modified-strong-collision matrix went singular (I-023). Fix: every QM conformer -- transition states AND any isomer whose ARC statmech artifact already exists -- is extracted through Arkane into vibration-only inline data (E0, scaled frequencies, imaginary mode) on one self-consistent energy reference, then spliced inline. Wells without an artifact keep their RMG estimate, which is already vibration-only, so a mixed-provenance network is still DOF-consistent. A write-time guard refuses any generated network whose isomers or TSs still carry a translational/rotational mode. - t3/runners/statmech_conformer_extract.py: Arkane-side driver (rmg_env) that loads an artifact and emits vibration-only conformer data as JSON. - t3/pdep/dof_conformers.py: orchestrates that driver from t3_env. - t3/pdep/hybrid.py: writer now consumes conformer-data dicts, splices TS and adopted wells inline vibration-only, drops the by-reference qm/ vendoring, and self-checks DOF consistency. - t3/pdep/pes_qm.py, t3/main.py: extract conformers (and, in the loop, locate well artifacts) before the write. Verified end to end under rmg_env_official: the real explorer crashes with the singular matrix on the old hybrid and reaches a round-1 rate coefficient on the new one. tests/test_pdep green (1910 passed), including a real-hybrid mixed-case test.

…letes The PES loop's round-1 re-exploration solved the master equation and produced a real rate coefficient, then tripped the explorer's own output-vs-network consistency check. Round 1 explores a hybrid whose species labels already carry one RMG disambiguation index (O=C=O(5)); Arkane appends a SECOND index to the network file it writes (O=C=O(5)(3)) while output.py keeps the single-indexed label. `_check_final_network_payload` normalizes both sides with `strip_rmg_index_suffix`, which stripped only the LAST `(<int>)` group -- leaving output at the base label and the network file one index level above it, so every species read as undeclared and a genuinely single-run pairing was refused. Strip every trailing index group down to the base (SMILES) identity, so indices that accumulate one-per-round collapse consistently on both sides. This never merges two real species: a SMILES base never itself ends in `(<int>)`. Not a loosening of the guard -- it still refuses networks with distinct species; it corrects the identity the guard compares on. The fix touches only label normalization; the round-1 rate coefficient (k_rev TST A=8.73653e+11, n=0.308264, Ea=106.537; TST+T A=0.201422, n=3.90893, Ea=71.7449) is unchanged, as Arkane writes it before the check runs. Adds a real-data test driving run_pes_loop's own round-0 -> round-1 path through the real explorer and the real committed CHO2 hybrid (QM replayed from committed conformers, not submitted), plus the accumulated-index cases to the parser test.

…and a closed file handle Three CodeQL reports on this branch, all real and all cheap: * a parametrize list used adjacent-literal concatenation, which is indistinguishable from a missing comma and would silently split one case in two -- made explicit with +; * a test imported t3.pdep.explorer.input_file both with 'import' and 'import from' purely to monkeypatch an attribute. Patched by string target instead, which is what every other test in the file already does; * statmech_conformer_extract.main() opened the spec file inside json.load(...) and never closed it.

`re` and `shutil` in tests/test_pdep/test_hybrid.py. The only remaining mention of shutil is inside a docstring, so neither is referenced.

…ression The handoff -- T3 splices QM'd conformers onto RMG estimates to write a hybrid Arkane network, then round 1 re-explores it through the real Arkane explorer -- was just driven from eight consecutive contact-defects to zero, and nothing protected it. Every one of those eight was live while the whole suite was green, because each lived in the contact between a producer and a consumer that no test ever put together on a real artifact (three were literally "the loop wrote a file its own reader rejected"). Add a single slow (~30s) end-to-end test that REGENERATES the real committed hybrid via write_hybrid_network_input_file and drives it through the real explorer (rmg_env), then asserts, one test each off a module-scoped fixture: round 1 completes; the reverse rate coefficient matches the measured Arrhenius fit within a documented 1% tolerance; the generated hybrid carries no translational/rotational mode (the singular-master-equation invariant, checked by an independent AST scan so it is not redundant with the writer's own guard); and the final network parses with the loop's own parser. Regenerates rather than replays a stored artifact -- a replay can only exercise code downstream of where the artifact was produced. Marked slow and skipped only where rmg_env is genuinely absent (never silently); runs in the normal tests/test_pdep suite where rmg_env exists, as CI must keep it.

…t inline TS folding TestArcQmRunnerRealCaptureHybrid drives arc_qm_runner's real capture+hybrid path, which now runs the DOF-conformer extraction: it shells into rmg_env and drives Arkane over the fabricated artifact's Log(...) reference. The placeholder output.out fails Arkane's ess_factory, so all five tests raised RuntimeError before reaching their assertions -- which also still encoded the pre-DOF by-reference TS folding (transitionState('TS1', 'qm/TS1.py') + a vendored qm/TS1.py carrying Log()), a format the DOF change replaced with a self-contained inline vibration-only block. - Vendor a real, completed Gaussian TS freq log (nC3H7 -> iC3H7, C3H7 doublet, one imaginary frequency) from ARC's test data, kept as .out so the *.log gitignore rule does not swallow it; the fabrication helper copies it in place of the placeholder. - Update the two stale assertions to the inline form the writer now emits: TS1's own transitionState block carries HarmonicOscillator modes and an imaginary frequency, and the hybrid references no external qm/ file or Log(). The DOF invariant is untouched.

This was referenced Aug 23, 2026

fix(explorer): let the outbound self-check accept the modelChemistry line the inbound guard admits #200

Merged

fix(explorer): absolutize spliced stat-mech file paths to the source's directory #201

Merged

alongd requested a lite review from Copilot August 24, 2026 16:45

alongd force-pushed the i018-modelchem branch from e2ce706 to 0208fad Compare August 24, 2026 16:45

Copilot started reviewing on behalf of alongd August 24, 2026 16:45 View session

Copilot AI reviewed Aug 24, 2026

View reviewed changes

Comment thread t3/pdep/explorer/input_file.py Outdated

Comment thread t3/pdep/explorer/input_file.py Outdated

Comment thread tests/test_pdep/test_explorer_input_file.py Outdated

alongd force-pushed the i018-modelchem branch from 0208fad to ab11884 Compare August 25, 2026 06:22

alongd force-pushed the i019-seed-label branch from b0be7fb to 0b8dec1 Compare August 25, 2026 06:29

Base automatically changed from i018-modelchem to main August 25, 2026 08:44

alongd force-pushed the i019-seed-label branch from 0b8dec1 to 5d5cb14 Compare August 25, 2026 08:44

github-advanced-security Bot found potential problems Aug 25, 2026

View reviewed changes

Comment thread tests/test_pdep/test_explorer_input_file.py Fixed

Comment thread tests/test_pdep/test_explorer_input_file.py Fixed

github-advanced-security Bot found potential problems Aug 25, 2026

View reviewed changes

Comment thread t3/runners/statmech_conformer_extract.py Fixed

alongd added 7 commits August 25, 2026 21:46

Drop two unused imports flagged by ruff

e0e9e13

`re` and `shutil` in tests/test_pdep/test_hybrid.py. The only remaining mention of shutil is inside a docstring, so neither is referenced.

alongd force-pushed the i019-seed-label branch from 89c86b0 to 36e8e4d Compare August 25, 2026 19:46

github-advanced-security Bot found potential problems Aug 25, 2026

View reviewed changes

Comment thread tests/test_pdep/test_round1_regression.py Fixed

Comment thread tests/test_pdep/test_round1_regression.py Fixed

Comment thread tests/test_pdep/test_round1_regression.py Fixed

Comment thread tests/test_pdep/test_round1_regression.py Fixed

alongd added 2 commits August 26, 2026 12:24

alongd force-pushed the i019-seed-label branch from 1c9af96 to c758604 Compare August 26, 2026 09:24

alongd merged commit b39ac60 into main Aug 26, 2026
4 checks passed

alongd deleted the i019-seed-label branch August 26, 2026 10:53

Navigation Menu

Uh oh!

fix(pdep): resolve the explorer seed to the round's own species label - #199

fix(pdep): resolve the explorer seed to the round's own species label#199
alongd merged 9 commits into
mainfrom
i019-seed-label

alongd commented Aug 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

alongd commented Aug 23, 2026

What this fixes

The change

Testing

Known next defect, pinned rather than chased

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Aug 25, 2026 •

edited

Loading