Skip to content

Commit 5f2138a

Browse files
authored
env_run: isolate adapter subprocesses from arc_env activation vars (#883)
## Summary ARC runs inside `arc_env`, but the AutoTST, GCN, and TorchANI adapters shell out to scripts that live in their own envs (`tst_env`, `ts_gcn_env`, `tani_env`). The previous code path was: ```python subprocess.run("source ~/.bashrc; <target_env_python> <script>", shell=True) ``` This invokes the target env's interpreter directly without deactivating `arc_env` first. ARC's exported activation vars (`BABEL_LIBDIR`, `LD_LIBRARY_PATH`, `CONDA_PREFIX`, ...) stay bound to `arc_env`'s paths in the child, so shared libraries in the target env then resolve plugins against the wrong tree — producing ABI-mismatch crashes (most visibly OpenBabel plugin loading in `tst_env`). ## What this PR does - Adds `arc/job/env_run.py` exposing `run_in_conda_env(python, script, *args)`, which routes the subprocess through `<launcher> run -n <env>` so the target env's `activate.d` hooks fire and rebind the leaked vars to the correct paths. - Detects an available launcher in preference order: the active one per `CONDA_EXE`/`MAMBA_EXE`, then `conda` → `mamba` → `micromamba` on PATH. Selects the right stdio flag per launcher (`--no-capture-output` for conda/mamba, omitted for micromamba which rejects it). - Wires the three callers (`arc/job/adapters/torch_ani.py`, `arc/job/adapters/ts/autotst_ts.py`, `arc/job/adapters/ts/gcn_ts.py`) over to the helper and drops the now-unused `subprocess` imports. - Adds `from __future__ import annotations` to `autotst_script.py` and `tani_script.py` so their PEP 604 union syntax (`X | Y`) imports cleanly under whatever Python version the target env ships (tst_env is 3.9; tani_env varies). ## Test plan - [x] AutoTST TS-search run completes without OpenBabel plugin errors - [x] GCN TS-search run completes and produces guess xyz - [x] TorchANI single-point job completes and writes output yml - [x] Verified on a host where `conda` is the active launcher; verify also under `mamba` / `micromamba` if available
2 parents 23bf4ae + d233064 commit 5f2138a

8 files changed

Lines changed: 419 additions & 19 deletions

File tree

arc/job/adapters/scripts/autotst_script.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
should be run under the tst_env.
77
"""
88

9+
# tst_env is Python 3.9; this script uses PEP 604 union syntax
10+
# (``str | None``) for parity with the rest of ARC. The future-import
11+
# defers annotation evaluation so 3.9 doesn't choke at def-time.
12+
from __future__ import annotations
13+
914
import argparse
1015
import numpy as np
1116
import os

arc/job/adapters/scripts/tani_script.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
should be run under the tani environment.
77
"""
88

9+
# tani_env's Python may predate PEP 604 (``X | Y`` unions); this
10+
# future-import defers annotation evaluation so the script imports on
11+
# any Python ≥3.7 regardless of the env's interpreter version.
12+
from __future__ import annotations
13+
914
import argparse
1015
import os
1116
import yaml

arc/job/adapters/torch_ani.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99
import datetime
1010
import os
1111
from typing import TYPE_CHECKING
12-
import subprocess
1312

1413
from arc.common import ARC_PATH, get_logger, is_xyz_linear, save_yaml_file, read_yaml_file
1514
from arc.imports import settings
1615
from arc.job.adapter import JobAdapter
1716
from arc.job.adapters.common import _initialize_adapter
17+
from arc.job.env_run import run_in_conda_env
1818
from arc.job.factory import register_job_adapter
1919
from arc.level import Level
2020
from arc.settings.settings import tani_default_options_dict
@@ -255,11 +255,12 @@ def execute_incore(self):
255255
return
256256

257257
self.write_input_file(tani_default_options_dict)
258-
commands = ['source ~/.bashrc',
259-
f'{TANI_PYTHON} {TANI_SCRIPT_PATH} '
260-
f'--yml_path {self.local_path}']
261-
command = '; '.join(commands)
262-
output = subprocess.run(command, shell=True, executable='/bin/bash')
258+
# Routed via run_in_conda_env so arc_env's activation vars don't
259+
# leak into the child (see arc/job/env_run.py).
260+
output = run_in_conda_env(
261+
TANI_PYTHON, TANI_SCRIPT_PATH,
262+
'--yml_path', self.local_path,
263+
)
263264
if output.returncode:
264265
logger.warning(f'Torch ANI subprocess ran and did not '
265266
f'give a successful return code for {self.job_name}.\n'

arc/job/adapters/ts/autotst_ts.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66

77
import datetime
88
import os
9-
import subprocess
109
from typing import TYPE_CHECKING
1110

1211
from arc.common import almost_equal_coords, ARC_PATH, get_logger, read_yaml_file
1312
from arc.imports import settings
1413
from arc.job.adapter import JobAdapter
1514
from arc.job.adapters.common import _initialize_adapter
15+
from arc.job.env_run import run_in_conda_env
1616
from arc.job.factory import register_job_adapter
1717
from arc.plotter import save_geo
1818
from arc.reaction import ARCReaction
@@ -237,14 +237,16 @@ def execute_incore(self):
237237

238238
i = 0
239239
for reaction_label, direction in zip([reaction_label_fwd, reaction_label_rev], ['F', 'R']):
240-
# run AutoTST as a subprocess in the desired direction
240+
# Run AutoTST as a subprocess in the desired direction.
241+
# run_in_conda_env keeps arc_env's activation vars
242+
# (BABEL_LIBDIR, LD_LIBRARY_PATH, ...) from leaking into the
243+
# child and corrupting openbabel plugin loading
244+
# (see arc/job/env_run.py).
241245
script_path = os.path.join(ARC_PATH, 'arc', 'job', 'adapters', 'scripts', 'autotst_script.py')
242-
commands = ['source ~/.bashrc', f'"{AUTOTST_PYTHON}" "{script_path}" "{reaction_label}" "{self.output_path}"']
243-
command = '; '.join(commands)
244246

245247
tic = datetime.datetime.now()
246248

247-
output = subprocess.run(command, shell=True, executable='/bin/bash')
249+
output = run_in_conda_env(AUTOTST_PYTHON, script_path, reaction_label, self.output_path)
248250

249251
tok = datetime.datetime.now() - tic
250252

arc/job/adapters/ts/gcn_ts.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
import datetime
1111
import os
12-
import subprocess
1312
from typing import TYPE_CHECKING
1413

1514
from rdkit import Chem
@@ -18,6 +17,7 @@
1817
from arc.imports import settings
1918
from arc.job.adapter import JobAdapter
2019
from arc.job.adapters.common import _initialize_adapter
20+
from arc.job.env_run import run_in_conda_env
2121
from arc.job.factory import register_job_adapter
2222
from arc.plotter import save_geo
2323
from arc.species.converter import rdkit_conf_from_mol, str_to_xyz
@@ -366,13 +366,14 @@ def run_subprocess_locally(direction: str,
366366
index=len(ts_species.ts_guesses),
367367
)
368368
tsg.tic()
369-
commands = ['source ~/.bashrc',
370-
f'{TS_GCN_PYTHON} {GCN_SCRIPT_PATH} '
371-
f'--r_sdf_path {product_path} '
372-
f'--p_sdf_path {reactant_path} '
373-
f'--ts_xyz_path {ts_path}']
374-
command = '; '.join(commands)
375-
output = subprocess.run(command, shell=True, executable='/bin/bash')
369+
# Routed via run_in_conda_env so arc_env's activation vars don't
370+
# leak into the child (see arc/job/env_run.py).
371+
output = run_in_conda_env(
372+
TS_GCN_PYTHON, GCN_SCRIPT_PATH,
373+
'--r_sdf_path', product_path,
374+
'--p_sdf_path', reactant_path,
375+
'--ts_xyz_path', ts_path,
376+
)
376377
if output.returncode:
377378
logger.warning(f'GCN subprocess ran in the reverse direction did not '
378379
f'give a successful return code for {ts_species}.\n'

arc/job/env_run.py

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
"""Invoke a script under a sibling conda/mamba env, isolated from ARC's env.
2+
3+
ARC runs inside ``arc_env``. Several adapters (AutoTST, GCN, TorchANI)
4+
shell out to scripts that live in their *own* envs (``tst_env``,
5+
``ts_gcn``, ``tani_env``). Running the target env's ``python``
6+
binary directly leaves ARC's exported activation vars (``BABEL_LIBDIR``,
7+
``LD_LIBRARY_PATH``, ``CONDA_PREFIX``, ...) bound to ``arc_env``'s
8+
paths in the child, which causes ABI-mismatch crashes when shared
9+
libraries in the child resolve plugins against the wrong env's tree.
10+
11+
Routing through a launcher's ``run`` subcommand makes the launcher
12+
deactivate the caller env and re-activate the target, so the target
13+
env's own ``activate.d`` hooks fire and bind those vars to its paths.
14+
15+
Three launchers are supported, in preference order:
16+
17+
1. ``conda`` — needs ``--no-capture-output`` to avoid buffering child
18+
stdio.
19+
2. ``mamba`` — same parser as conda for ``run``; also needs
20+
``--no-capture-output``.
21+
3. ``micromamba`` — independent C++ reimplementation; streams stdio by
22+
default and **rejects** ``--no-capture-output``, so the flag must be
23+
omitted.
24+
25+
Buffering matters: without the right flag, conda/mamba hold the child's
26+
stdout until exit, hiding tracebacks and progress.
27+
28+
The launcher is detected at call time, with the active one (per
29+
``CONDA_EXE`` / ``MAMBA_EXE``) preferred when available.
30+
"""
31+
32+
import os
33+
import shutil
34+
import subprocess
35+
from pathlib import Path
36+
37+
from arc.common import get_logger
38+
39+
logger = get_logger()
40+
41+
42+
def env_prefix_from_python(python_executable: str) -> str:
43+
"""Derive the env prefix from an interpreter path.
44+
45+
ARC's settings expose target Python interpreters as full paths
46+
(``AUTOTST_PYTHON``, ``TS_GCN_PYTHON``, ``TANI_PYTHON``). The env
47+
prefix passed to ``<launcher> run -p <prefix>`` is the directory two
48+
levels above the binary (``<prefix>/bin/python``).
49+
50+
Using a prefix path rather than ``-n <name>`` avoids assuming the
51+
env lives under a literal ``envs/`` segment — ``CONDA_ENVS_PATH``
52+
and bare-prefix mamba/micromamba layouts (e.g.
53+
``/scratch/conda_envs/<env>/bin/python``) are both fine.
54+
55+
Validation is lexical, NOT through ``Path.resolve()``: in real
56+
conda/mamba/micromamba envs ``<prefix>/bin/python`` is a symlink to
57+
``python3.X``, so resolving first would replace the basename with
58+
``python3.12`` (or similar) and trip the name check. The launcher
59+
follows its own interpreter, so all we need here is the prefix
60+
string the caller already gave us.
61+
"""
62+
path = Path(python_executable)
63+
if path.name != "python" or path.parent.name != "bin":
64+
raise ValueError(
65+
f"Cannot derive an env prefix from {python_executable!r}; "
66+
"expected a path of the form '<prefix>/bin/python'."
67+
)
68+
return str(path.parent.parent)
69+
70+
71+
def _run_flags_for(launcher_path: str) -> list[str]:
72+
"""Return the per-launcher flags needed for ``run`` to stream stdio.
73+
74+
Decided by the launcher's basename rather than which env var pointed
75+
us at it, so symlinks and odd ``MAMBA_EXE``-points-at-micromamba
76+
setups still get the right flag.
77+
"""
78+
name = Path(launcher_path).name
79+
if name == "micromamba":
80+
return []
81+
return ["--no-capture-output"]
82+
83+
84+
def _detect_launcher() -> tuple[str, list[str]]:
85+
"""Return ``(launcher_path, extra_run_flags)``.
86+
87+
Preference: whichever launcher is active in the current shell
88+
(``CONDA_EXE`` / ``MAMBA_EXE``), then conda → mamba → micromamba on
89+
PATH.
90+
"""
91+
for env_var in ("CONDA_EXE", "MAMBA_EXE"):
92+
path = os.environ.get(env_var)
93+
if path and os.path.isfile(path):
94+
return path, _run_flags_for(path)
95+
for name in ("conda", "mamba", "micromamba"):
96+
found = shutil.which(name)
97+
if found:
98+
return found, _run_flags_for(found)
99+
raise FileNotFoundError(
100+
"No conda-family launcher (conda / mamba / micromamba) found on "
101+
"PATH. ARC's cross-env adapters (AutoTST/GCN/TorchANI) need one "
102+
"of these to launch their subprocess scripts in isolated envs."
103+
)
104+
105+
106+
def run_in_conda_env(
107+
python_executable: str,
108+
script_path: str,
109+
*script_args: str,
110+
check: bool = False,
111+
) -> subprocess.CompletedProcess:
112+
"""Run ``python script_path *script_args`` inside the env that owns
113+
``python_executable``, isolated from ARC's process env.
114+
115+
stdout and stderr are captured and logged centrally — debug on
116+
success, warning (with both streams and the return code) on
117+
non-zero exit — so call sites don't each re-implement capture and
118+
error reporting. The captured streams are also exposed on the
119+
returned :class:`subprocess.CompletedProcess` (``.stdout`` /
120+
``.stderr``) for callers that need to inspect them. ``check=True``
121+
raises ``CalledProcessError`` on non-zero exit. Args are passed as
122+
a list, so no shell quoting concerns.
123+
"""
124+
env_prefix = env_prefix_from_python(python_executable)
125+
launcher, extra_flags = _detect_launcher()
126+
argv = [
127+
launcher, "run", *extra_flags,
128+
"-p", env_prefix,
129+
"python", script_path,
130+
*script_args,
131+
]
132+
result = subprocess.run(argv, check=check, capture_output=True, text=True)
133+
if result.returncode:
134+
logger.warning(
135+
"env-run: %s exited with %d\ncmd: %s\nstdout:\n%s\nstderr:\n%s",
136+
script_path, result.returncode, " ".join(argv),
137+
result.stdout, result.stderr,
138+
)
139+
else:
140+
logger.debug(
141+
"env-run: %s exited 0\ncmd: %s\nstdout:\n%s\nstderr:\n%s",
142+
script_path, " ".join(argv), result.stdout, result.stderr,
143+
)
144+
return result

0 commit comments

Comments
 (0)