Skip to content

TS GCN - Fix#920

Open
calvinp0 wants to merge 4 commits into
mainfrom
gcn_fix
Open

TS GCN - Fix#920
calvinp0 wants to merge 4 commits into
mainfrom
gcn_fix

Conversation

@calvinp0

Copy link
Copy Markdown
Member

This pull request refactors and extends the gcn_script.py script to improve its usability, error handling, and compatibility with the TS-GCN environment. The script now supports both direct and batch execution modes, improves inference import logic, and provides clearer error messages and exit codes.

Major improvements:

1. Direct and Batch Execution Modes

  • The script now supports two modes: direct mode (using SDF file arguments for one-off execution) and batch mode (using a YAML input file for queue execution). Command-line arguments and the main function were updated to allow both workflows. [1] [2] [3]

2. Robust TS-GCN Inference Import

  • Added the import_inference function to robustly locate and import the TS-GCN inference module, searching well-known locations and providing clear error messages if not found. [1] [2]

3. Improved Error Handling and Exit Codes

  • The script now checks for inference success and file existence, prints detailed error messages (including tracebacks), and exits with appropriate exit codes on failure or argument errors. [1] [2] [3]

4. API and Documentation Updates

  • Updated function docstrings for clarity, fixed typos, and clarified argument descriptions throughout the script. [1] [2] [3]

5. Code Cleanup and Structure

  • Removed unused imports, ensured that ARC modules are not imported, and improved code structure for maintainability. [1] [2]

These changes make the script more robust, flexible, and easier to use in different environments.

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.

calvinp0 added 4 commits July 22, 2026 14:06
The GCN TS-search integration was broken end to end:
- gcn_ts.py gated on a module-level 'from inference import inference'
  (HAS_GCN), which can never succeed inside arc_env; the flag was also
  never consumed. Remove it: availability is now determined by the
  subprocess boundary being usable (gcn_available(): TS_GCN_PYTHON
  resolves to an existing interpreter and gcn_script.py exists), and
  execute_gcn() logs a clear warning and skips GCN TS guesses instead
  of crashing the scheduler when the ts_gcn env is missing
  (previously run_in_conda_env(None, ...) raised a TypeError).
- run_subprocess_locally() passed the reactant SDF to --p_sdf_path and
  the product SDF to --r_sdf_path (inverting both search directions),
  and its failure log always claimed the reverse direction.
- gcn_script.py only accepted --yml_in_path, so every incore invocation
  exited 2 with an argparse error. It now supports both the direct
  per-direction mode (--r_sdf_path/--p_sdf_path/--ts_xyz_path) used by
  incore execution and the YAML batch mode used by queue execution,
  defers the TS-GCN 'inference' import to call time with a sys.path
  fallback ($TSGCN_ROOT, then the TS-GCN clone next to ARC) since
  'conda run' sources neither ~/.bashrc nor login shells, and replaces
  a bare except that swallowed inference failures while exiting 0.
- The queue submit templates activated arc_env (instead of ts_gcn) and
  invoked arc/job/adapters/ts/scripts/gcn_runner.py, a path that no
  longer exists since the scripts folder relocation.
- install_gcn.sh: the conda activation hook was written with a quoted
  heredoc delimiter, so $repo was never expanded and TSGCN_ROOT ended
  up empty at activation time; the PYTHONPATH case pattern also lacked
  a trailing wildcard. Expand install-time values, escape runtime ones.
- gcn_environment.yml (used by install_gcn_cpu.sh) contained no
  torch / torch-geometric at all, so the ts_gcn env it created could
  not run TS-GCN inference. Pin python=3.8 and add the same CPU wheel
  pins install_gcn.sh already uses.
Mock run_in_conda_env at the env boundary and assert the exact argv
(interpreter, gcn_script.py, --r_sdf_path/--p_sdf_path/--ts_xyz_path
per direction), the SDF input files written, and the parsing of a fake
TS xyz output into TSGuess objects. Cover the failing-returncode path
and the graceful skip when the ts_gcn env is unavailable
(TS_GCN_PYTHON=None must not spawn a subprocess or raise).
Add plumbing tests for the standalone gcn_script.py (CLI modes, batch
YAML output, informative ImportError, no arc imports).
Use per-test directories with addCleanup since the suite runs under
pytest-xdist (worksteal), where class-scoped shared directories race
across workers.
…s_gcn hook

Unpinned 'pip install torch-geometric' pulled 2.6.x (needs torch>=2.0, imports
torch.utils._pytree absent in the pinned torch 1.7.1) -> TS-GCN ModuleNotFoundError.
Pin to torch-1.7.1-compatible 1.7.2. Also export LD_LIBRARY_PATH=$CONDA_PREFIX/lib
in the activation hook so scipy finds the conda libstdc++ (GLIBCXX_3.4.29) rather than
an older system /lib64 one (surfaced on zeus once the torch-geometric import was fixed).
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.

2 participants