CREST Adapter #807

calvinp0 · 2025-11-27T22:16:29Z

Addition of CREST Adapter that complements the heuristic adapter.

Fixed Arkane to check LoT by ignoring the year suffix (plans to implement year in the input) AutoTST does not attempt to import autotst (due to its reliance on RMG-Py env).

arc/job/adapters/ts/crest.py

arc/settings/settings.py

arc/job/adapters/ts/autotst_ts.py

arc/job/adapters/ts/crest.py

arc/job/adapters/ts/heuristics.py

arc/job/adapters/ts/crest.py

Copilot

Pull request overview

This PR adds a CREST adapter to generate transition state (TS) conformer guesses for H_Abstraction reactions, complementing the existing heuristic approach. The integration includes CREST path detection, conda environment setup, and job submission/monitoring workflows. Several installation scripts were improved with better environment management, PATH/PYTHONPATH handling, and more robust activation/deactivation hooks. The Arkane module gained flexible model chemistry matching that handles hyphenated method names and year suffixes.

Key Changes:

New CREST adapter module (arc/job/adapters/ts/crest.py) integrating constrained TS conformer searches into the heuristics workflow
Enhanced Arkane model chemistry lookup with normalization and year-aware matching for RMG quantum corrections database
Improved installation scripts with better conda environment isolation and conflict resolution (AutoTST/RMG-Py, GCN)

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`arc/job/adapters/ts/crest.py`	New CREST adapter with job setup, submission, monitoring, and H-abstraction atom detection
`arc/job/adapters/ts/heuristics.py`	Integrates CREST TS search alongside heuristics, submits jobs and processes results
`arc/job/adapters/ts/crest_test.py`	Unit tests for CREST input file generation
`arc/statmech/arkane.py`	Adds flexible matching for LevelOfTheory strings, normalizing hyphens and handling year suffixes
`arc/species/converter.py`	New `reorder_xyz_string` function for coordinate format/unit conversion with deprecation wrapper
`arc/species/converter_test.py`	Tests for the new XYZ reordering function
`arc/settings/settings.py`	CREST executable discovery across conda/mamba/micromamba environments and standalone builds
`devtools/install_crest.sh`	New installer for CREST 2.12 via conda
`devtools/install_autotst.sh`	Enhanced PATH management to isolate AutoTST from RMG-Py conflicts
`devtools/install_rmg.sh`	Improved PATH/PYTHONPATH additions with duplicate prevention
`devtools/install_gcn.sh`	Fixed activation hooks and switched to `conda run` for pip installs
`devtools/install_pyrdl.sh`	Corrected cmake installation to target arc_env explicitly
`devtools/install_torchani.sh`	Added LOGFILE variable for cleaner log handling
`devtools/install_all.sh`	Added `--no-rmg` flag, reordered PyRDL after ARC, added CREST to external tools
`devtools/crest_environment.yml`	Conda environment specification for CREST 2.12
`Makefile`	Added `install-crest` target
`AGENTS.md`	Repository guidelines file (added but also gitignored - should be removed)
`.gitignore`	Ignores AGENTS.md
`arc/species/species.py`	Added logging for similar TS guesses during clustering
`arc/job/adapters/ts/autotst_ts.py`	Better error handling for missing AutoTST package in subprocess

Comments suppressed due to low confidence (1)

arc/job/adapters/ts/heuristics.py:289

When CREST generates TS guesses, they will have different timing information than the heuristics guesses, but all guesses (both Heuristics and CREST) are being assigned tsg.t0 and tsg.execution_time from the heuristics TSGuess object. CREST guesses should have their own timing information, or at minimum the execution_time should reflect the actual CREST job duration, not the heuristics generation time.

                                       t0=tsg.t0,
                                       execution_time=tsg.execution_time,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-28T13:01:48Z

arc/job/adapters/ts/crest.py

+            atom for atom in closest_atoms if not atom.startswith("H")
+        )
+        if_hydrogen_present = any(
+            atom
+            for atom in closest_atoms


The logic in get_h_abs_atoms is incorrect. Line 305-312 checks any(atom for atom in closest_atoms if not atom.startswith("H")) which iterates over keys of closest_atoms dict (strings like "H0", "C1"), not the values. It should check any(atom for atom in atom_neighbours if not atom.startswith("H")) to check the actual neighbors of the hydrogen. The same issue exists on lines 308-312 for if_hydrogen_present.

Suggested change

atom for atom in closest_atoms if not atom.startswith("H")

)

if_hydrogen_present = any(

atom

for atom in closest_atoms

atom for atom in atom_neighbours if not atom.startswith("H")

)

if_hydrogen_present = any(

atom

for atom in atom_neighbours

Copilot · 2025-11-28T13:01:48Z

arc/job/adapters/ts/crest.py

+                "H": extract_digits(best_occurrence["H"]),
+                "A": extract_digits(best_occurrence["A"]),
+                "B": extract_digits(best_occurrence["B"]),
+            }


When len(condition_occurrences) == 1, the function should return the single occurrence but doesn't. After the if len(condition_occurrences) > 1: block completes at line 341, control falls through to the else block at line 342. The single occurrence case needs to be handled explicitly, or the logic restructured to return condition_occurrences[0] when there's exactly one match.

Suggested change

}

}

elif len(condition_occurrences) == 1:

occurrence = condition_occurrences[0]

return {

"H": extract_digits(occurrence["H"]),

"A": extract_digits(occurrence["A"]),

"B": extract_digits(occurrence["B"]),

}

Copilot · 2025-11-28T13:01:48Z

arc/species/converter.py

+
+    Args:
+        xyz_str (str): The string xyz format to be converted.
+        reverse_atoms (bool, optional): Whether to reverse the atoms and coordinates.


The docstring is missing the units parameter which is part of the function signature. This parameter should be documented to specify the units of the input coordinates (either 'angstrom' or 'bohr').

Suggested change

reverse_atoms (bool, optional): Whether to reverse the atoms and coordinates.

reverse_atoms (bool, optional): Whether to reverse the atoms and coordinates.

units (str, optional): The units of the input coordinates (either 'angstrom' or 'bohr').

Copilot · 2025-11-28T13:01:49Z

arc/job/adapters/ts/crest.py

+            }
+    else:
+
+        # Check the all the hydrogen atoms, and see the closest two heavy atoms and aggregate their distances to determine which Hyodrogen atom has the lowest distance aggregate


Spelling error: "Hyodrogen" should be "Hydrogen".

Suggested change

# Check the all the hydrogen atoms, and see the closest two heavy atoms and aggregate their distances to determine which Hyodrogen atom has the lowest distance aggregate

# Check the all the hydrogen atoms, and see the closest two heavy atoms and aggregate their distances to determine which Hydrogen atom has the lowest distance aggregate

Copilot · 2025-11-28T13:01:49Z

devtools/install_autotst.sh

+autotst_on () {
+    export AUTOTST_ROOT="__AUTOTST_PATH__"
+    export AUTOTST_OLD_PATH="$PATH"
+    export AUTOTST_OLD_PYTHONPATH="${PYTHONPATH:-}"
+
+    _strip_path () { local needle=":$1:"; local haystack=":$2:"; echo "${haystack//$needle/:}" | sed 's/^://;s/:$//'; }


The _strip_path function is defined inside the heredoc with double-quoted variables, but the function body uses single quotes for string operations. This creates a fragile quoting situation. Consider defining this as a regular shell function outside the heredoc, or ensure consistent quoting to avoid unexpected variable expansion issues.

Suggested change

autotst_on () {

export AUTOTST_ROOT="__AUTOTST_PATH__"

export AUTOTST_OLD_PATH="$PATH"

export AUTOTST_OLD_PYTHONPATH="${PYTHONPATH:-}"

_strip_path () { local needle=":$1:"; local haystack=":$2:"; echo "${haystack//$needle/:}" | sed 's/^://;s/:$//'; }

_strip_path () { local needle=":$1:"; local haystack=":$2:"; echo "${haystack//$needle/:}" | sed 's/^://;s/:$//'; }

autotst_on () {

export AUTOTST_ROOT="__AUTOTST_PATH__"

export AUTOTST_OLD_PATH="$PATH"

export AUTOTST_OLD_PYTHONPATH="${PYTHONPATH:-}"

Copilot · 2025-11-28T13:01:50Z

arc/job/adapters/ts/crest.py

+                    distances = dataframe.loc[hydrogen_key, heavy_atoms].sum()
+                    occurrence_distances.append((occurrence, distances))
+                except KeyError as e:
+                    print(f"Error accessing distances for occurrence {occurrence}: {e}")


Use logger.error() instead of print() for consistency with the rest of the codebase. The logger is already imported and used elsewhere in this module.

Suggested change

print(f"Error accessing distances for occurrence {occurrence}: {e}")

logger.error(f"Error accessing distances for occurrence {occurrence}: {e}")

Copilot · 2025-11-28T13:01:50Z

arc/statmech/arkane.py

+
+def _iter_level_keys_from_section(file_path: str,
+                                  section_start: str,
+                                  section_end: str) -> list[str]:


The type hint list[str] requires Python 3.9+. Since this project supports Python 3.7+ (as seen in environment files), you need to either use List[str] from typing (which is already imported on line 9) or add from __future__ import annotations at the top of the file.

Suggested change

section_end: str) -> list[str]:

section_end: str) -> List[str]:

Copilot · 2025-11-28T13:01:51Z

arc/statmech/arkane.py

+    return method.lower().replace('-', '')
+
+
+def _split_method_year(method_norm: str):


The return type annotation is missing for _split_method_year. Based on the docstring examples and implementation, it should return Tuple[str, Optional[int]].

Copilot · 2025-11-28T13:01:51Z

arc/job/adapters/ts/autotst_ts.py

+# try:
+#     from autotst.reaction import Reaction as AutoTST_Reaction
+# except (ImportError, ModuleNotFoundError):
+#     HAS_AUTOTST = False



This comment appears to contain commented-out code.

Suggested change

# try:

# from autotst.reaction import Reaction as AutoTST_Reaction

# except (ImportError, ModuleNotFoundError):

# HAS_AUTOTST = False

Copilot · 2025-11-28T13:01:51Z

arc/job/adapters/ts/heuristics.py

-
 import datetime
 import itertools
+import os


Import of 'os' is not used.

Suggested change

import os

arc/job/adapters/ts/crest.py

+        else:
+            raise ValueError("No valid hydrogen atom found.")
+
+    return {}


The best fix is to delete the final unreachable return {} at line 378 in the get_h_abs_atoms function in arc/job/adapters/ts/crest.py. All execution paths in the function already either return with a dictionary or raise an exception. Removing this line does not alter any functionality, as it cannot be executed under any code path. No imports, method or variable definitions, or other changes are needed—just removal of this single line.

calvinp0 added 2 commits November 27, 2025 21:57

Added Crest into heuristics

8c1ef14

Fixed Arkane to check LoT by ignoring the year suffix (plans to implement year in the input) AutoTST does not attempt to import autotst (due to its reliance on RMG-Py env).

Crest Adapter

734db3e

github-actions bot added Module: Converter Module: Species labels Nov 27, 2025

github-advanced-security bot found potential problems Nov 27, 2025

View reviewed changes

calvinp0 added 3 commits November 28, 2025 00:19

Added AGENTS.md to gitignore

8c11525

ARC Installation fixes

8f2d147

Added CREST test and also fixed CodeQL errors

d2f5e03

calvinp0 requested a review from Copilot November 28, 2025 12:56

Copilot started reviewing on behalf of calvinp0 November 28, 2025 12:56 View session

github-advanced-security bot found potential problems Nov 28, 2025

View reviewed changes

arc/job/adapters/ts/crest.py Fixed Show fixed Hide fixed

Copilot finished reviewing on behalf of calvinp0 November 28, 2025 13:00

Copilot AI reviewed Nov 28, 2025

View reviewed changes

calvinp0 added 4 commits November 28, 2025 15:02

Minor fixes

f421c04

Stop tracking AGENTS.md

152dcbb

Installation documentation update

55eb8ec

Update fixes

7b89b6e

github-advanced-security bot found potential problems Nov 28, 2025

View reviewed changes

-            }
+            }
+        elif len(condition_occurrences) == 1:
+            occurrence = condition_occurrences[0]
+            return {
+                "H": extract_digits(occurrence["H"]),
+                "A": extract_digits(occurrence["A"]),
+                "B": extract_digits(occurrence["B"]),
+            }

	reverse_atoms (bool, optional): Whether to reverse the atoms and coordinates.
	reverse_atoms (bool, optional): Whether to reverse the atoms and coordinates.
	units (str, optional): The units of the input coordinates (either 'angstrom' or 'bohr').

	# Check the all the hydrogen atoms, and see the closest two heavy atoms and aggregate their distances to determine which Hyodrogen atom has the lowest distance aggregate
	# Check the all the hydrogen atoms, and see the closest two heavy atoms and aggregate their distances to determine which Hydrogen atom has the lowest distance aggregate

	print(f"Error accessing distances for occurrence {occurrence}: {e}")
	logger.error(f"Error accessing distances for occurrence {occurrence}: {e}")

	section_end: str) -> list[str]:
	section_end: str) -> List[str]:

		return method.lower().replace('-', '')


		def _split_method_year(method_norm: str):

CREST Adapter #807

Are you sure you want to change the base?

CREST Adapter #807

Conversation

calvinp0 commented Nov 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Check warning

Copilot Autofix

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants