Skip to content

Kinbot env - #918

Merged
calvinp0 merged 13 commits into
mainfrom
kinbot_env
Jul 27, 2026
Merged

Kinbot env#918
calvinp0 merged 13 commits into
mainfrom
kinbot_env

Conversation

@calvinp0

@calvinp0 calvinp0 commented Jul 21, 2026

Copy link
Copy Markdown
Member

This pull request refactors and modernizes the KinBotAdapter integration and its tests in ARC, moving from a direct KinBot API usage to a subprocess-based approach that invokes KinBot in a dedicated conda environment. The test suite is rewritten to use unittest and mock, adding robust coverage for subprocess boundaries, error handling, and configuration options.

The most important changes are:

Adapter Refactor and Subprocess Integration

  • The KinBotAdapter is refactored to run KinBot via a subprocess in its own conda environment, using a standalone worker script (kinbot_script.py). The adapter now checks for the presence of the KinBot Python executable and raises a clear error if missing, removing the old direct API and HAS_KINBOT checks. [1] [2]
  • Paths to input and output YAML files are now set up in the adapter, facilitating communication with the KinBot worker script.

Test Suite Rewrite and Expansion

  • The test suite in kinbot_test.py is rewritten to use unittest and mock, with helper functions for test isolation and subprocess mocking. Tests now cover family mapping, subprocess error handling, parsing of results, UMA refinement flags, and end-to-end integration with a real KinBot environment. [1] [2]

Settings and Configuration

  • The adapter now loads configuration for the KinBot Python executable and UMA settings from settings, ensuring consistent environment management.

Code Cleanup

  • Removes all direct KinBot imports and the HAS_KINBOT logic, further decoupling ARC from KinBot internals.

Documentation

  • Updates module-level and class docstrings to reflect the new subprocess-based approach and clarify test coverage. [1] [2]

These changes improve maintainability, testability, and reliability of KinBot integration in ARC, and make the adapter more robust to environment and dependency issues.

Copilot AI review requested due to automatic review settings July 21, 2026 18:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread arc/job/adapters/ts/kinbot_test.py Fixed
Comment thread arc/job/adapters/ts/kinbot_test.py Fixed
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.42%. Comparing base (10c8e96) to head (5b52752).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #918      +/-   ##
==========================================
+ Coverage   63.40%   63.42%   +0.02%     
==========================================
  Files         114      114              
  Lines       38342    38325      -17     
  Branches    10030    10030              
==========================================
  Hits        24309    24309              
+ Misses      11116    11098      -18     
- Partials     2917     2918       +1     
Flag Coverage Δ
functionaltests 63.42% <ø> (+0.02%) ⬆️
unittests 63.42% <ø> (+0.02%) ⬆️

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.

@kfir4444 kfir4444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @calvinp0! Overall lgtm, see minor comments.

Comment thread arc/settings/settings.py Outdated
Comment thread arc/job/adapters/scripts/kinbot_script.py
Comment thread arc/job/adapters/scripts/kinbot_script.py Outdated
calvinp0 added 10 commits July 22, 2026 23:52
KinBot is no longer installed into arc_env. The installer now creates a
dedicated kinbot_env conda environment (Python 3.12) and installs current
upstream KinBot (2.3.0, which requires Python >= 3.11) via pip, as
recommended upstream, along with PyYAML for ARC's worker script IO.
The worker script runs inside kinbot_env and imports only KinBot, PyYAML,
and the standard library. It reads a YAML input file (wells with SMILES,
KinBot-format structures, charge, multiplicity, and families), reproduces
the TS guess generation logic previously inlined in kinbot_ts.py
(ReactionFinder/ReactionGenerator + get_constraints + modify_coordinates),
and writes the resulting guesses to a YAML output file, mirroring
gcn_script.py's conventions.

Adaptations to KinBot 2.3.0: the 'dimer' and 'homolytic_scissions'
parameters no longer exist and are not set (an explicit families list is
always passed, so homolytic scissions cannot be triggered).

KINBOT_PYTHON resolves the kinbot_env python analogously to TS_GCN_PYTHON.
The KinBot TS search adapter no longer imports kinbot into arc_env.
It now writes a YAML input file describing the unimolecular wells,
invokes arc/job/adapters/scripts/kinbot_script.py inside kinbot_env via
run_in_conda_env (the same pattern as the GCN and AutoTST adapters), and
parses the worker's YAML output into TSGuess objects exactly as before
(collision check, deduplication, save_geo, method indexing).

Availability is now determined by the existence of the KINBOT_PYTHON
executable rather than by importing kinbot into arc_env.
The tests no longer require KinBot in arc_env (they were previously
skipped silently unless kinbot was importable). run_in_conda_env is
mocked to assert the input file contents and to emulate the worker's
output file, covering: the happy path, a crashed subprocess, result
parsing (duplicates merged, failures kept as unsuccessful guesses,
unknown directions skipped), the family map, and the missing
KINBOT_PYTHON availability check. Project directory cleanup uses
addCleanup.
Since KinBot 2.2, Parameters validates its values at construction time and
exits unless a barrier threshold is set, so the worker now supplies its
parameter overrides (including a nominal barrier_threshold, unused in
ARC's flow since no KinBot QC jobs are spawned) through a temporary JSON
input file instead of mutating params.par after construction.

Verified end-to-end against a locally built kinbot_env (KinBot 2.3.0):
the adapter produces the same TS guesses through the subprocess boundary
as the previous in-arc_env integration did.

Also point the TS search docs at the kinbot_env environment.
Step-semantics fix: previously (both in this integration and in the
pre-existing in-arc_env one) get_constraints() was called once with step=20,
which exceeds every KinBot family's max_step, so the 'change' constraint
list was always empty and the returned TS guesses were unmodified well
geometries. The worker now drives each reaction instance exactly like
KinBot's reac_family.carry_out_reaction() does between the
constrained-optimization jobs it submits: step from 0 (or 12 for the
short-instance skip shortcut) through max_step - 1, applying each step's
'change' constraints (including the 'L'-prefixed entry conversion) to the
evolving geometry via modify_coordinates. There is no step magic number
anymore; the 'step' input key and KINBOT_CONSTRAINT_STEP were removed.

Verified end-to-end against the real kinbot_env: CC[O] <=> [CH2]CO now
yields 10 distinct TS guesses displaced 0.5-3.7 A from their wells (all
previously equaled the well geometries). A kinbot_env-gated regression
test asserts the guess geometries differ from the wells. Test project
directories are now unique per test with per-directory cleanup, so
parallel pytest-xdist workers cannot delete each other's directories.

UMA refinement (opt-in, off by default): the worker can refine each
successful template guess with a Sella saddle-point search on Meta's UMA
machine-learned potential, mirroring KinBot 2.3.0's ase_fc_ts_end
template. fairchem is lazily imported inside the refinement function only,
and any refinement failure (missing fairchem, gated/missing checkpoint,
non-convergence) logs the reason and falls back to the unrefined guess.
Controlled by the new 'kinbot_uma_settings' dict in settings.py, passed
through input.yml; refined guesses are labeled method 'KinBot-UMA' (still
containing 'kinbot', keeping dedup and success-count logic intact) and
output entries carry an explicit 'uma_refined' flag for benchmarking.
install_kinbot.sh --uma installs KinBot with its 'fc' pip extra
(fairchem-core, for the opt-in UMA refinement of TS guesses) and
sanity-checks the fairchem import. The script header documents that the
UMA checkpoints themselves are license-gated on HuggingFace (they require
a HuggingFace login and accepting Meta's UMA license, or a locally
downloaded checkpoint file), which is why the default install stays lean.
…; log teardown OSError instead of silent pass)
…lt uma-s-1p2 latest-small; uma-m-1p1 selectable; uma-s-1p1 legacy)

@kfir4444 kfir4444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! good job again @calvinp0!

@calvinp0
calvinp0 merged commit 9787770 into main Jul 27, 2026
8 checks passed
@calvinp0
calvinp0 deleted the kinbot_env branch July 27, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants