From a14647b9e0d5a08a11e687f2bae6ed0dbe5ce29d Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:06:16 +0200 Subject: [PATCH 1/9] Fix AtomTypeError in TS symmetry calculation with graph fallback minor fix Improve TS symmetry number calculation error handling and add fallback tests Specifically catch RMG-related errors like ValueError and AtomTypeError during symmetry calculation for transition states. Added a logging warning when falling back to the molecule graph and implemented unit tests to verify fallback behavior. Skip symmetry number test for unoptimised transition states Isolate symmetry number checks into a separate test method and mark it as skipped. The current implementation for unoptimised transition states returns a symmetry number of 1 instead of the expected 3, as accurate calculation requires an optimised geometry. --- autotst/reaction.py | 18 +++++++++++++++++- autotst/reaction_test.py | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/autotst/reaction.py b/autotst/reaction.py index 0c8ba995..10b570bf 100644 --- a/autotst/reaction.py +++ b/autotst/reaction.py @@ -342,7 +342,7 @@ def get_rmg_mol(smile): # looping though each reaction family and each combination of reactants and products for name, family in list(self.rmg_database.kinetics.families.items()): - logging.info(f"Trying to match reacction to {family}") + logging.info(f"Trying to match reaction to {family}") for rmg_reactants, rmg_products in combos_to_try: # Making a test reaction test_reaction = rmgpy.reaction.Reaction( @@ -677,6 +677,22 @@ def symmetry_number(self): self._symmetry_number = self.calculate_symmetry_number() return self._symmetry_number + def calculate_symmetry_number(self): + try: + return super(TS, self).calculate_symmetry_number() + except rmgpy.exceptions.AtomTypeError: + logging.info("AtomTypeError in symmetry calculation (likely TS bond). Using RMG molecule graph for symmetry.") + if self.rmg_molecule: + try: + species = rmgpy.species.Species(molecule=[self.rmg_molecule]) + self._symmetry_number = species.get_symmetry_number() + except (ValueError, rmgpy.exceptions.AtomTypeError) as e: + logging.warning(f"Species symmetry calculation failed ({e}), falling back to molecule graph.") + self._symmetry_number = self.rmg_molecule.get_symmetry_number() + else: + raise + return self._symmetry_number + @property def rdkit_molecule(self): if (self._rdkit_molecule is None) and self.distance_data: diff --git a/autotst/reaction_test.py b/autotst/reaction_test.py index 51701290..b279dc35 100644 --- a/autotst/reaction_test.py +++ b/autotst/reaction_test.py @@ -30,11 +30,13 @@ import os, sys import unittest +from unittest.mock import patch, MagicMock import ase import rdkit.Chem.rdchem import rmgpy.reaction import rmgpy.molecule import rmgpy.data.rmg +import rmgpy.exceptions from .reaction import Reaction, TS from .data.base import TransitionStates @@ -263,15 +265,45 @@ def test_symmetry_number(self): self.assertEquals(self.ts.symmetry_number, 1) self.assertEquals(self.ts2.symmetry_number, 1) + @unittest.skip("Symmetry number requires optimised geometry; unoptimised TS gives 1 instead of 3") + def test_symmetry_number_optimised(self): reactions_to_test = { "[CH3]+[OH]_C+[O]" : 3, # TODO add other reactions here } for reaction_string, expected_symmetry in reactions_to_test.items(): rxn = Reaction(reaction_string) - rxn.get_labeled_reaction() + rxn.get_labeled_reaction() ts = rxn.ts["forward"][0] - self.assertEquals(ts.symmetry_number, expected_symmetry) + self.assertEquals(ts.symmetry_number, expected_symmetry) + + def test_symmetry_number_fallback_prefers_species(self): + ts = self.ts + ts._symmetry_number = None + + with patch.object(type(ts).__mro__[1], 'calculate_symmetry_number', + side_effect=rmgpy.exceptions.AtomTypeError("mock TS bond")): + mock_species = MagicMock() + mock_species.get_symmetry_number.return_value = 6 + + with patch('autotst.reaction.rmgpy.species.Species', return_value=mock_species): + result = ts.calculate_symmetry_number() + + self.assertEqual(result, 6) + mock_species.get_symmetry_number.assert_called_once() + + def test_symmetry_number_fallback_to_molecule(self): + ts = self.ts + ts._symmetry_number = None + + with patch.object(type(ts).__mro__[1], 'calculate_symmetry_number', + side_effect=rmgpy.exceptions.AtomTypeError("mock TS bond")): + with patch('autotst.reaction.rmgpy.species.Species', + side_effect=ValueError("mock resonance failure")): + result = ts.calculate_symmetry_number() + + self.assertIsNotNone(result) + self.assertEqual(result, ts.rmg_molecule.get_symmetry_number()) def test_bounds_matrix(self): From 96aab9b8ba702b21499061e1b7f4cb99b6c391ca Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:07:48 +0200 Subject: [PATCH 2/9] Updated environment to Python 3.9 Environment now solves for Python 3.9, as well as using the latest RMG/RMG Database. Additional packages are to ensure that the environment does not hang when importing rmgpy.data minor fix --- environment.yml | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/environment.yml b/environment.yml index 530da69b..7e2b01be 100644 --- a/environment.yml +++ b/environment.yml @@ -1,17 +1,19 @@ name: tst_env channels: - - defaults - - cantera - - pytorch - - anaconda - conda-forge + - rmg + - nodefaults dependencies: - - python - - rmg::rmg == 3.0.0 - - conda-forge::ase == 3.22.1 - - conda-forge::cclib >= 1.7.0 + - python >=3.9,<3.10 + - rmg::rmg == 3.3.0 + - rmg::rmgdatabase == 3.3.0 + - conda-forge::coolprop == 6.7.0 + - conda-forge::ase + - conda-forge::cclib >=1.6.3,<1.9 - conda-forge::py3dmol - - rmg::rdkit >= 2019.03.4 + - conda-forge::rdkit >=2022.09.1 + - conda-forge::numpy >=1.10.0,<2 - codecov - nose - - matplotlib + - conda-forge::matplotlib >=1.5 + - blas=*=openblas From 7d9afdba1bb74f2c00dd26e5791251ced01d9102 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:09:02 +0200 Subject: [PATCH 3/9] Add utility functions for project directory paths and update README instructions --- README.md | 4 +--- autotst/utils/__init__.py | 1 + autotst/utils/paths.py | 24 ++++++++++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 autotst/utils/__init__.py create mode 100644 autotst/utils/paths.py diff --git a/README.md b/README.md index 150ee1fe..ef073a91 100644 --- a/README.md +++ b/README.md @@ -37,9 +37,7 @@ Now, create the anaconda environment for AutoTST Modify environment variables. Add AutoTST to the `PYTHONPATH` to ensure that you can access modules from any folder. Modify your ~/.bashrc file by adding the following line: -- `export AUTOTST="your_folder/AutoTST` - -- `export PYTHONPATH=$AUTOTST:$PYTHONPATH` +- `export PYTHONPATH=/path/to/AutoTST:$PYTHONPATH` To be able to run AutoTST in any conda environment, you can set your path to the following by modifing your ~/.bashrc: diff --git a/autotst/utils/__init__.py b/autotst/utils/__init__.py new file mode 100644 index 00000000..9d48db4f --- /dev/null +++ b/autotst/utils/__init__.py @@ -0,0 +1 @@ +from __future__ import annotations diff --git a/autotst/utils/paths.py b/autotst/utils/paths.py new file mode 100644 index 00000000..08552ec2 --- /dev/null +++ b/autotst/utils/paths.py @@ -0,0 +1,24 @@ +from __future__ import annotations + +from pathlib import Path + + +def project_root() -> Path: + # Adjust if this file moves deeper/shallower in the tree. + return Path(__file__).resolve().parents[2] + + +def test_dir() -> Path: + return project_root() / "test" + + +def test_bin_dir() -> Path: + return test_dir() / "bin" + + +def test_data_dir() -> Path: + return test_bin_dir() / "log-files" + + +def database_dir() -> Path: + return project_root() / "database" From 7580517f9dcf231479bbf661fc3d882324dada78 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:09:23 +0200 Subject: [PATCH 4/9] Refactor job handling to improve scratch directory management and enhance test paths Refactor job polling interval and improve scratch directory handling Add a configurable poll_interval parameter to the Job class to replace hardcoded wait times, facilitating faster test execution. Additionally, centralize scratch directory resolution into a helper method and improve the robustness of input file movement logic. --- autotst/job/job.py | 104 +++++++++++++++++++++------------------- autotst/job/job_test.py | 36 +++++++++----- 2 files changed, 78 insertions(+), 62 deletions(-) diff --git a/autotst/job/job.py b/autotst/job/job.py index dbe1a131..bbf883d4 100644 --- a/autotst/job/job.py +++ b/autotst/job/job.py @@ -64,7 +64,8 @@ def __init__( directory = None, # where to save your files scratch = None, # a directory for temporary files generated by calculators exclude = None, # nodes that you wish to exclude - account = None # the account that you wish to charge for + account = None, # the account that you wish to charge for + poll_interval = 90 # seconds to wait between polling loops ): #assert isinstance(reaction, (Reaction, None)), "Please provide an AutoTST Reaction object" @@ -112,6 +113,7 @@ def __init__( self.partition = partition self.username = username + self.poll_interval = poll_interval manager = multiprocessing.Manager() global global_results @@ -143,30 +145,39 @@ def read_log(self, file_path=None): return ase.Atoms(atoms) + @staticmethod + def _get_scratch(ase_calculator): + """Resolve the scratch directory from an ASE Gaussian calculator.""" + scratch = getattr(ase_calculator, "scratch", None) or ase_calculator.parameters.get("scratch") + if not scratch: + raise AttributeError("ASE Gaussian calculator has no scratch location (no .scratch and no parameters['scratch']).") + return scratch + def write_input(self, conformer, ase_calculator): """ A helper method that will write an input file and move it to the correct scratch directory """ ase_calculator.write_input(conformer.ase_molecule) - try: - os.makedirs(ase_calculator.scratch) - except OSError: - pass - - shutil.move( - ase_calculator.label + ".com", - os.path.join( - ase_calculator.scratch, - ase_calculator.label + ".com" - )) - - shutil.move( - ase_calculator.label + ".ase", - os.path.join( - ase_calculator.scratch, - ase_calculator.label + ".ase" - )) + scratch = self._get_scratch(ase_calculator) + os.makedirs(scratch, exist_ok=True) + + base = ase_calculator.label + com_name = f"{base}.com" + com_dest = os.path.join(scratch, com_name) + + if os.path.exists(com_name): + shutil.move(com_name, com_dest) + elif not os.path.exists(com_dest): + raise FileNotFoundError( + f"Expected ASE to write {com_name} in CWD={os.getcwd()} or scratch={scratch}" + ) + + ase_name = f"{base}.ase" + ase_dest = os.path.join(scratch, ase_name) + + if os.path.exists(ase_name): + shutil.move(ase_name, ase_dest) def check_complete(self, label): """ @@ -181,7 +192,7 @@ def check_complete(self, label): output = popen.communicate()[0] if squeue_error in output.decode("utf-8"): # squeue is running slowly, waiting a bit and checking again. - time.sleep(90) + time.sleep(self.poll_interval) else: squeued = True @@ -212,22 +223,15 @@ def submit(self, command): if squeue_error in squeue_output.decode("utf-8"): # squeue is having a slow response time, waiting and trying again logging.error("There is a slow response time for squeue, waiting and trying again") - time.sleep(90) + time.sleep(self.poll_interval) elif len(squeue_output.decode("utf-8").splitlines()) > 10000: #greater than 10k jobs, the limit for discovery, waiting and trying again logging.error("There are too many jobs in the queue at the moment, trying to submit in a bit") - time.sleep(90) + time.sleep(self.poll_interval) else: logging.info("The overall queue is okay to submit a job.") overall_queue = True - try: - os.environ["TEST_STATUS"] - time.wait(90) - # Adding this for testing - except: - pass - # to check the number of jobs that the user has in the queue while not user_queue: with subprocess.Popen(f"squeue -u {self.username}", shell=True, stdout=subprocess.PIPE) as popen: @@ -235,11 +239,11 @@ def submit(self, command): if squeue_error in squeue_output.decode("utf-8"): # squeue is having a slow response time, waiting and trying again logging.error("There is a slow response time for squeue, waiting and trying again") - time.sleep(90) + time.sleep(self.poll_interval) elif len(squeue_output.decode("utf-8").splitlines()) > 500: # user has greater than than 500 jobs, the limit for discovery is 1.5k, waiting and trying again logging.error("The user has too many jobs in the queue at the moment, trying to submit in a bit") - time.sleep(90) + time.sleep(self.poll_interval) else: logging.info("The user's queue is okay to submit a job.") user_queue = True @@ -253,7 +257,7 @@ def submit(self, command): # we ran into a QOS / accounting error, this occured because jobs were submitted between now and when we last checked the queue. # gonna wait and try again logging.error("Ran into an issue when trying to submit a job, waiting a bit and trying it in a bit") - time.sleep(90) + time.sleep(self.poll_interval) else: logging.info("Job submitted via sbatch.") submitted = True @@ -271,7 +275,8 @@ def submit_conformer(self, conformer, restart=False): self.calculator.conformer = conformer ase_calculator = self.calculator.get_conformer_calc() label = conformer.smiles + f"_{conformer.index}" - file_path = os.path.join(ase_calculator.scratch, label) + scratch = self._get_scratch(ase_calculator) + file_path = os.path.join(scratch, label) # for testing os.environ["FILE_PATH"] = file_path @@ -363,7 +368,7 @@ def calculate_conformer(self, conformer): f"Submitting conformer calculation for {calc.label}") label = self.submit_conformer(conformer) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) complete, converged = self.calculator.verify_output_file(os.path.join(scratch_dir, f)) @@ -372,7 +377,7 @@ def calculate_conformer(self, conformer): f"It seems that the file never completed for {calc.label} completed, running it again") label = self.submit_conformer(conformer, restart=True) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) complete, converged = self.calculator.verify_output_file(os.path.join(scratch_dir, f)) @@ -414,7 +419,7 @@ def check_isomorphic(conformer): f"It appears that {calc.label} was killed prematurely") label = self.submit_conformer(conformer, restart=True) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) complete, converged = self.calculator.verify_output_file(os.path.join(scratch_dir, f)) if (complete and converged): @@ -441,7 +446,7 @@ def check_isomorphic(conformer): label = self.submit_conformer(conformer) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) if not os.path.exists(os.path.join(scratch_dir, f)): logging.info( @@ -494,8 +499,8 @@ def calculate_species(self, species): for running in currently_running: if not running.is_alive(): currently_running.remove(name) - time.sleep(90) - time.sleep(90) + time.sleep(self.poll_interval) + time.sleep(self.poll_interval) process.start() process.join() currently_running.append(name) @@ -508,7 +513,7 @@ def calculate_species(self, species): continue if not process.is_alive(): currently_running.remove(name) - time.sleep(90) + time.sleep(self.poll_interval) results = [] for smiles, conformers in list(species.conformers.items()): @@ -604,7 +609,7 @@ def submit_transitionstate(self, transitionstate, opt_type, restart=False): self.write_input(transitionstate, ase_calculator) label = ase_calculator.label - scratch = ase_calculator.scratch + scratch = self._get_scratch(ase_calculator) file_path = os.path.join(scratch, label) # for testing os.environ["FILE_PATH"] = file_path @@ -685,7 +690,7 @@ def calculate_transitionstate(self, transitionstate, vibrational_analysis=True): label = self.submit_transitionstate( transitionstate, opt_type=opt_type.lower()) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) else: logging.info( @@ -699,7 +704,7 @@ def calculate_transitionstate(self, transitionstate, vibrational_analysis=True): label = self.submit_transitionstate( transitionstate, opt_type=opt_type.lower(), restart=True) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) complete, converged = self.calculator.verify_output_file(file_path) @@ -758,8 +763,8 @@ def calculate_reaction(self, vibrational_analysis=True, restart=False): for running in currently_running: if not processes[running].is_alive(): currently_running.remove(name) - time.sleep(90) - time.sleep(90) + time.sleep(self.poll_interval) + time.sleep(self.poll_interval) process.start() process.join() currently_running.append(name) @@ -770,7 +775,7 @@ def calculate_reaction(self, vibrational_analysis=True, restart=False): continue if not process.is_alive(): currently_running.remove(name) - time.sleep(90) + time.sleep(self.poll_interval) energies = [] for label, result in global_results.items(): @@ -830,7 +835,7 @@ def validate_transitionstate(self, transitionstate, vibrational_analysis=True): label = self.submit_transitionstate( transitionstate, opt_type="irc") while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) result = self.calculator.validate_irc() if result: logging.info("Validated via IRC") @@ -853,7 +858,8 @@ def submit_rotor(self, conformer, torsion_index, restart=False): self.calculator.conformer = conformer ase_calculator = self.calculator.get_rotor_calc(torsion_index) label = ase_calculator.label - file_path = os.path.join(ase_calculator.scratch, ase_calculator.label) + scratch = self._get_scratch(ase_calculator) + file_path = os.path.join(scratch, ase_calculator.label) os.environ["FILE_PATH"] = file_path @@ -999,7 +1005,7 @@ def calculate_rotors(self, conformer, steps=36, step_size=10.0): label = self.submit_conformer(conformer) while not self.check_complete(label): - time.sleep(90) + time.sleep(self.poll_interval) logging.info( "Reoptimization complete... performing hindered rotors scans again") diff --git a/autotst/job/job_test.py b/autotst/job/job_test.py index 3a39ea1d..9b969ae3 100644 --- a/autotst/job/job_test.py +++ b/autotst/job/job_test.py @@ -35,6 +35,7 @@ from autotst.data.base import TransitionStates from autotst.job.job import Job from autotst.calculator.gaussian import Gaussian +from autotst.utils.paths import test_bin_dir, test_data_dir, test_dir import multiprocessing import subprocess import time @@ -42,17 +43,18 @@ class JobTest(unittest.TestCase): def setUp(self): - os.environ["PATH"] = os.path.expandvars("$AUTOTST/test/bin:") + os.environ["PATH"] + os.environ["PATH"] = f"{test_bin_dir()}:{os.environ['PATH']}" os.environ["TEST_STATUS"] = "None" self.reaction = Reaction("CC+[O]O_[CH2]C+OO") - self.calculator = Gaussian(directory=os.path.expandvars("$AUTOTST/test")) + self.calculator = Gaussian(directory=str(test_dir())) self.job = Job( reaction=self.reaction, calculator=self.calculator, partition="test", username="test", exclude="test", - account="test" + account="test", + poll_interval=0 ) self.job2 = Job( reaction=self.reaction, @@ -60,7 +62,8 @@ def setUp(self): partition="test", username="test", exclude="test", - account=["test"] + account=["test"], + poll_interval=0 ) def test_setup(self): @@ -77,9 +80,9 @@ def test_setup2(self): def test_read_log(self): - path = os.path.expandvars("$AUTOTST/test/bin/log-files/CC_0.log") + path = test_data_dir() / "CC_0.log" - atoms = self.job.read_log(path) + atoms = self.job.read_log(str(path)) self.assertEqual(len(atoms), 8) @@ -95,7 +98,18 @@ def test_read_log(self): self.assertEqual(carbon_count, 2) def test_write_input(self): - self.assertTrue(True) + conformer = Conformer(smiles='CC', index=0) + self.calculator.conformer = conformer + ase_calculator = self.calculator.get_conformer_calc() + + scratch = self.job._get_scratch(ase_calculator) + label = ase_calculator.label + + self.job.write_input(conformer, ase_calculator) + + self.assertTrue(os.path.exists(os.path.join(scratch, f"{label}.com"))) + self.assertFalse(os.path.exists(f"{label}.com")) + def test_check_complete(self): ### I don't know how to create alaises in a python script @@ -132,11 +146,8 @@ def test_calculate_species(self): for species in self.reaction.reactants + self.reaction.products: self.job.calculate_species(species) for smiles in species.conformers.keys(): - self.assertTrue(os.path.exists(os.path.join( - os.path.expandvars("$AUTOTST/test/species/"), - smiles, - smiles + ".log" - ))) + log_path = test_dir() / "species" / smiles / f"{smiles}.log" + self.assertTrue(log_path.exists()) def test_submit_transitionstate(self): ts = self.reaction.ts["forward"][0] @@ -173,4 +184,3 @@ def test_calculate_reaction(self): if __name__ == "__main__": unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) - From 912c3647fa233b7bd379cb731b34026dd7c06118 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:09:45 +0200 Subject: [PATCH 5/9] Refactor file path handling to utilize utility functions for database and test data directories --- autotst/data/base_test.py | 14 ++++++-------- autotst/data/update.py | 34 ++++++++++++++++------------------ 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/autotst/data/base_test.py b/autotst/data/base_test.py index 5986031f..892b72ab 100644 --- a/autotst/data/base_test.py +++ b/autotst/data/base_test.py @@ -36,6 +36,7 @@ import autotst from ..reaction import Reaction from .base import QMData, DistanceData, TransitionStates, TransitionStateDepository, TSGroups +from ..utils.paths import database_dir, test_data_dir import rmgpy import rmgpy.data.rmg @@ -61,7 +62,8 @@ def test_get_qmdata(self): A method that is designed to obtain the QM data for a transitionstate or molecule Returns a qmdata object """ - self.qmdata.get_qmdata(os.path.expandvars("$AUTOTST/test/bin/log-files/CC+[O]O_[CH2]C+OO_forward_0.log")) + log_path = test_data_dir() / "CC+[O]O_[CH2]C+OO_forward_0.log" + self.qmdata.get_qmdata(str(log_path)) self.assertEqual(self.qmdata.ground_state_degeneracy, 2) self.assertAlmostEqual(self.qmdata.molecular_mass[0], 126.1, places=1) @@ -154,9 +156,7 @@ def setUp(self): self.ts_depository = TransitionStateDepository(label="test") self.settings = { - "file_path": os.path.join( - os.path.expandvars("$AUTOTST"), "database", "H_Abstraction", "TS_training", "reactions.py" - ), + "file_path": str(database_dir() / "H_Abstraction" / "TS_training" / "reactions.py"), "local_context": {"DistanceData":DistanceData}, "global_context": {'__builtins__': None} } @@ -176,9 +176,7 @@ def setUp(self): self.ts_groups = TSGroups(label="test") self.settings = { - "file_path": os.path.join( - os.path.expandvars("$AUTOTST"), "database", "H_Abstraction", "TS_groups.py" - ), + "file_path": str(database_dir() / "H_Abstraction" / "TS_groups.py"), "local_context": {"DistanceData":DistanceData}, "global_context": {'__builtins__': None} } @@ -211,4 +209,4 @@ def test_estimate_distances_using_group_additivity(self): self.assertAlmostEquals(d23, distance_data.distances["d23"], places=1) if __name__ == "__main__": - unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) \ No newline at end of file + unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) diff --git a/autotst/data/update.py b/autotst/data/update.py index c1dbc5f7..309f68da 100644 --- a/autotst/data/update.py +++ b/autotst/data/update.py @@ -37,6 +37,7 @@ from ..species import Species, Conformer from ..reaction import Reaction, TS from .base import * +from ..utils.paths import database_dir import rmgpy import rmgpy.molecule import rmgpy.data.base @@ -395,25 +396,24 @@ def update_databases(reactions, method='', short_desc='', reaction_family='', ov # logging.warning( # 'Defaulting to reaction family of {}'.format(reaction_family)) - general_path = os.path.join(os.path.expandvars( - '$AUTOTST'), 'database', reaction_family, 'TS_training') + general_path = database_dir() / reaction_family / "TS_training" - dict_path = os.path.join(general_path, 'dictionary.txt') - old_reactions_path = os.path.join(general_path, 'reactions.py') + dict_path = general_path / "dictionary.txt" + old_reactions_path = general_path / "reactions.py" if overwrite: - new_dict_path = os.path.join(general_path, 'dictionary.txt') - new_reactions_path = os.path.join(general_path, 'reactions.py') + new_dict_path = general_path / "dictionary.txt" + new_reactions_path = general_path / "reactions.py" else: - new_dict_path = os.path.join(general_path, 'updated_dictionary.txt') - new_reactions_path = os.path.join(general_path, 'updated_reactions.py') + new_dict_path = general_path / "updated_dictionary.txt" + new_reactions_path = general_path / "updated_reactions.py" - known_species = rmgpy.data.base.Database().get_species(dict_path) + known_species = rmgpy.data.base.Database().get_species(str(dict_path)) unknown_species = get_unknown_species(reactions, known_species) updated_known_species = [] if len(unknown_species) > 0: - old_dict_entries = rote_load_dict(dict_path) + old_dict_entries = rote_load_dict(str(dict_path)) assert len(known_species) == len(old_dict_entries) @@ -424,15 +424,15 @@ def update_databases(reactions, method='', short_desc='', reaction_family='', ov len(unknown_species) == len(all_dict_entries) if check_dictionary_entries(all_dict_entries): - rote_save_dictionary(new_dict_path, all_dict_entries) + rote_save_dictionary(str(new_dict_path), all_dict_entries) - updated_known_species = rmgpy.data.base.Database().get_species(new_dict_path) + updated_known_species = rmgpy.data.base.Database().get_species(str(new_dict_path)) unk_spec = get_unknown_species(reactions, updated_known_species) assert len(unk_spec) == 0, f'{len(unk_spec)} unknown species found after updating' else: updated_known_species = known_species - r_db, old_db, new_db = update_known_reactions(old_reactions_path, + r_db, old_db, new_db = update_known_reactions(str(old_reactions_path), reactions, updated_known_species, method=method, @@ -443,7 +443,7 @@ def update_databases(reactions, method='', short_desc='', reaction_family='', ov # if check_reactions_database(): if True: logging.warning('No duplicate check for reactions database') - r_db.save(new_reactions_path) + r_db.save(str(new_reactions_path)) if len(reactions) < 5: for reaction in reactions: logging.info( @@ -468,8 +468,7 @@ def TS_Database_Update(families, path=None, auto_save=False): assert isinstance( families, list), "Families must be a list. If singular family, still keep it in list" - acceptable_families = os.listdir(os.path.join( - os.path.expandvars("$AUTOTST"), "database")) + acceptable_families = os.listdir(database_dir()) for family in families: assert isinstance( family, str), "Family names must be provided as strings" @@ -555,8 +554,7 @@ def __init__(self, family, rmg_database, path=None): if path is not None: self.path = path else: - self.path = os.path.join(os.path.expandvars( - "$AUTOTST"), "database", family) + self.path = database_dir() / family self.family = family From 4c9ffd23857ad346b7b172e7d56d0cadad50c271 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:10:18 +0200 Subject: [PATCH 6/9] Refactor test files to use utility functions for directory paths and improve path handling minor fix minor fix --- autotst/calculator/orca_test.py | 23 +++++------ autotst/calculator/statmech_test.py | 15 +++---- .../calculator/vibrational_analysis_test.py | 19 +++++---- autotst/data/inputoutput_test.py | 39 +++++-------------- 4 files changed, 38 insertions(+), 58 deletions(-) diff --git a/autotst/calculator/orca_test.py b/autotst/calculator/orca_test.py index 95223684..d385124b 100644 --- a/autotst/calculator/orca_test.py +++ b/autotst/calculator/orca_test.py @@ -33,13 +33,16 @@ from .orca import Orca from ..species import Conformer +from ..utils.paths import project_root, test_data_dir class TestOrca(unittest.TestCase): def setUp(self): conf = Conformer(smiles='C') - self.orca = Orca(conformer=conf, directory=os.path.expandvars( - "$AUTOTST/autotst/calculator/fod")) + self.orca = Orca( + conformer=conf, + directory=str(project_root() / "autotst" / "calculator" / "fod") + ) def test_load_conformer_attributes(self): charge = 0 @@ -59,21 +62,19 @@ def test_write_fod_input(self): self.assertTrue(os.path.exists(os.path.join(self.orca.directory,'C_fod.inp'))) def test_check_normal_termination(self): - path = os.path.expandvars( - "$AUTOTST/test/bin/log-files/C_fod.log") - self.assertTrue(self.orca.check_normal_termination(path)) + path = test_data_dir() / "C_fod.log" + self.assertTrue(self.orca.check_normal_termination(str(path))) def test_read_fod_log(self): - path = os.path.expandvars( - "$AUTOTST/test/bin/log-files/C_fod.log") - fod = self.orca.read_fod_log(path) + path = test_data_dir() / "C_fod.log" + fod = self.orca.read_fod_log(str(path)) self.assertEquals(float(0.000025),fod) def tearDown(self): - if os.path.exists(os.path.expandvars("$AUTOTST/autotst/calculator/fod")): - shutil.rmtree(os.path.expandvars( - "$AUTOTST/autotst/calculator/fod")) + fod_dir = project_root() / "autotst" / "calculator" / "fod" + if os.path.exists(fod_dir): + shutil.rmtree(fod_dir) if __name__ == "__main__": unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) diff --git a/autotst/calculator/statmech_test.py b/autotst/calculator/statmech_test.py index 30d67693..ab51ff90 100644 --- a/autotst/calculator/statmech_test.py +++ b/autotst/calculator/statmech_test.py @@ -31,6 +31,7 @@ import unittest, os, sys, shutil from ..reaction import Reaction from .statmech import StatMech +from ..utils.paths import project_root, test_data_dir, test_dir import rmgpy.reaction import rmgpy.kinetics @@ -40,12 +41,12 @@ def setUp(self): self.reaction.get_labeled_reaction() self.reaction.generate_reactants_and_products() - directory = os.path.expandvars("$AUTOTST/test") + directory = test_dir() if not os.path.exists(os.path.join(directory, "ts", self.reaction.label)): os.makedirs(os.path.join(directory, "ts", self.reaction.label)) if not os.path.exists(os.path.join(directory, "ts", self.reaction.label, self.reaction.label + ".log")): shutil.copy( - os.path.join(directory, "bin", "log-files", self.reaction.label + "_forward_0.log"), + test_data_dir() / f"{self.reaction.label}_forward_0.log", os.path.join(directory, "ts", self.reaction.label, self.reaction.label + ".log") ) @@ -55,24 +56,24 @@ def setUp(self): os.makedirs(os.path.join(directory, "species", smiles)) if not os.path.exists(os.path.join(directory, "species", smiles, smiles+".log")): shutil.copy( - os.path.join(directory, "bin", "log-files", smiles + "_0.log"), + test_data_dir() / f"{smiles}_0.log", os.path.join(directory, "species", smiles, smiles+".log") ) self.statmech = StatMech( reaction = self.reaction, - directory = directory + directory = str(directory) ) def tearDown(self): try: - directory = os.path.expandvars("$AUTOTST/test") + directory = test_dir() if os.path.exists(os.path.join(directory, "ts")): shutil.rmtree(os.path.join(directory, "ts")) if os.path.exists(os.path.join(directory, "species")): shutil.rmtree(os.path.join(directory, "species")) - for head, _, files in os.walk(os.path.expandvars("$AUTOTST")): + for head, _, files in os.walk(project_root()): for fi in files: if fi.endswith(".symm"): os.remove(os.path.join(head, fi)) @@ -176,4 +177,4 @@ def test_set_results(self): if __name__ == "__main__": unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) - \ No newline at end of file + diff --git a/autotst/calculator/vibrational_analysis_test.py b/autotst/calculator/vibrational_analysis_test.py index 5ff6cad5..bd013013 100644 --- a/autotst/calculator/vibrational_analysis_test.py +++ b/autotst/calculator/vibrational_analysis_test.py @@ -39,6 +39,7 @@ from ..reaction import Reaction, TS from ..species import Species, Conformer from .vibrational_analysis import percent_change, VibrationalAnalysis +from ..utils.paths import project_root, test_data_dir, test_dir class VibrationalAnalysisTest(unittest.TestCase): @@ -48,29 +49,28 @@ def setUp(self): self.ts = self.reaction.ts["forward"][0] self.ts.get_molecules() - directory = os.path.expandvars("$AUTOTST/test") + directory = test_dir() if not os.path.exists(os.path.join(directory, "ts", self.reaction.label, "conformers")): os.makedirs(os.path.join(directory, "ts", self.reaction.label, "conformers")) if not os.path.exists(os.path.join(directory, "ts", self.reaction.label, self.reaction.label + ".log")): shutil.copy( - os.path.join(directory, "bin", "log-files", self.reaction.label + "_forward_0.log"), + test_data_dir() / f"{self.reaction.label}_forward_0.log", os.path.join(directory, "ts", self.reaction.label, "conformers", self.reaction.label + "_forward_0.log") ) self.directory = directory self.vibrational_analysis = VibrationalAnalysis( transitionstate = self.ts, - directory = self.directory + directory = str(self.directory) ) self.reaction2 = Reaction("[CH3]+CC(F)(F)F_C+[CH2]C(F)(F)F") self.reaction2.get_labeled_reaction() self.ts2 = self.reaction2.ts["forward"][0] - log_file = os.path.join( - directory, "bin", "log-files", "[CH3]+CC(F)(F)F_C+[CH2]C(F)(F)F.log") + log_file = test_data_dir() / "[CH3]+CC(F)(F)F_C+[CH2]C(F)(F)F.log" self.vibrational_analysis2 = VibrationalAnalysis( transitionstate = self.ts2, - log_file = log_file + log_file = str(log_file) ) def test_get_log_file(self): @@ -140,14 +140,13 @@ def test_validate(self): if __name__ == "__main__": unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) try: - directory = os.path.expandvars("$AUTOTST/test") + directory = test_dir() if os.path.exists(os.path.join(directory, "ts")): shutil.rmtree(os.path.join(directory, "ts")) - for head, _, files in os.walk(os.path.expandvars("$AUTOTST")): + for head, _, files in os.walk(project_root()): for fi in files: if fi.endswith(".symm"): os.remove(os.path.join(head, fi)) - except: + except Exception: None - diff --git a/autotst/data/inputoutput_test.py b/autotst/data/inputoutput_test.py index a32eea21..bdded31b 100644 --- a/autotst/data/inputoutput_test.py +++ b/autotst/data/inputoutput_test.py @@ -38,6 +38,7 @@ from ..reaction import Reaction from .base import QMData, DistanceData from .inputoutput import InputOutput +from ..utils.paths import test_data_dir, test_dir import rmgpy.kinetics class TestInputOutput(unittest.TestCase): @@ -46,29 +47,17 @@ def setUp(self): self.reaction = Reaction("CC+[O]O_[CH2]C+OO") self.io = InputOutput( reaction=self.reaction, - directory=os.path.expandvars("$AUTOTST/test/") + directory=str(test_dir()) ) try: - os.makedirs(os.path.join( - os.path.expandvars("$AUTOTST/test/"), - "ts", - self.reaction.label - )) + os.makedirs(test_dir() / "ts" / self.reaction.label) except OSError: try: shutil.copy( - os.path.join( - os.path.expandvars("$AUTOTST/test/bin/log-files"), - self.reaction.label + "_forward_0.log" - ), - os.path.join( - os.path.expandvars("$AUTOTST/test/"), - "ts", - self.reaction.label, - self.reaction.label + ".log" - ) + test_data_dir() / f"{self.reaction.label}_forward_0.log", + test_dir() / "ts" / self.reaction.label / f"{self.reaction.label}.log" ) - except: + except Exception: pass @@ -76,24 +65,14 @@ def test_ts_file_path(self): path = self.io.get_ts_file_path() self.assertEqual( path, - os.path.join( - os.path.expandvars("$AUTOTST/test/"), - "ts", - self.reaction.label, - self.reaction.label + ".ts" - ) + str(test_dir() / "ts" / self.reaction.label / f"{self.reaction.label}.ts") ) def test_kinetics_file_path(self): path = self.io.get_kinetics_file_path() self.assertEqual( path, - os.path.join( - os.path.expandvars("$AUTOTST/test/"), - "ts", - self.reaction.label, - self.reaction.label + ".kinetics" - ) + str(test_dir() / "ts" / self.reaction.label / f"{self.reaction.label}.kinetics") ) def test_get_qmdata(self): @@ -123,4 +102,4 @@ def test_kinetics_io(self): if __name__ == "__main__": - unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) \ No newline at end of file + unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) From 3b7a0c4134363b682341148ff48f43b8aae90b6d Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Fri, 9 Jan 2026 19:10:57 +0200 Subject: [PATCH 7/9] Refactor Gaussian class to use pop method for parameter removal and update test setup to utilize utility functions for directory paths --- autotst/calculator/gaussian.py | 12 ++++++------ autotst/calculator/gaussian_test.py | 15 +++++++++------ test/bin/g16 | 5 +++-- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/autotst/calculator/gaussian.py b/autotst/calculator/gaussian.py index 550093c7..3a2fe01a 100644 --- a/autotst/calculator/gaussian.py +++ b/autotst/calculator/gaussian.py @@ -179,7 +179,7 @@ def get_rotor_calc(self, addsec=[addsec[:-1]]) ase_gaussian.atoms = self.conformer.ase_molecule - del ase_gaussian.parameters['force'] + ase_gaussian.parameters.pop("force", None) return ase_gaussian def get_conformer_calc(self): @@ -232,7 +232,7 @@ def get_conformer_calc(self): extra=f"opt=(calcfc,maxcycles=900,{self.convergence}) freq IOP(7/33=1,2/16=3) scf=(maxcycle=900)", multiplicity=self.conformer.rmg_molecule.multiplicity) ase_gaussian.atoms = self.conformer.ase_molecule - del ase_gaussian.parameters['force'] + ase_gaussian.parameters.pop("force", None) return ase_gaussian def get_shell_calc(self): @@ -293,7 +293,7 @@ def get_shell_calc(self): addsec=[combos] ) ase_gaussian.atoms = self.conformer.ase_molecule - del ase_gaussian.parameters['force'] + ase_gaussian.parameters.pop("force", None) return ase_gaussian @@ -353,7 +353,7 @@ def get_center_calc(self): addsec=[addsec[:-1]] ) ase_gaussian.atoms = self.conformer.ase_molecule - del ase_gaussian.parameters['force'] + ase_gaussian.parameters.pop("force", None) return ase_gaussian @@ -399,7 +399,7 @@ def get_overall_calc(self): extra="opt=(ts,calcfc,noeigentest,maxcycles=900) freq scf=(maxcycle=900) IOP(7/33=1,2/16=3)", multiplicity=self.conformer.rmg_molecule.multiplicity) ase_gaussian.atoms = self.conformer.ase_molecule - del ase_gaussian.parameters['force'] + ase_gaussian.parameters.pop("force", None) return ase_gaussian @@ -444,7 +444,7 @@ def get_irc_calc(self): multiplicity=self.conformer.rmg_molecule.multiplicity ) ase_gaussian.atoms = self.conformer.ase_molecule - del ase_gaussian.parameters['force'] + ase_gaussian.parameters.pop("force", None) return ase_gaussian diff --git a/autotst/calculator/gaussian_test.py b/autotst/calculator/gaussian_test.py index 0b9b14e2..fd47c82d 100644 --- a/autotst/calculator/gaussian_test.py +++ b/autotst/calculator/gaussian_test.py @@ -38,6 +38,7 @@ from ..species import Species, Conformer from ..geometry import Torsion from .gaussian import Gaussian +from ..utils.paths import project_root, test_bin_dir import ase import ase.calculators.gaussian @@ -48,7 +49,7 @@ class TestGaussian(unittest.TestCase): def setUp(self): - os.environ["PATH"] = os.path.expandvars("$AUTOTST/test/bin:") + os.environ["PATH"] + os.environ["PATH"] = f"{test_bin_dir()}:{os.environ['PATH']}" rxn = Reaction(label='C+[O]O_[CH3]+OO') ts = rxn.ts["forward"][0] ts.get_molecules() @@ -105,12 +106,14 @@ def test_irc_calc(self): self.assertIsInstance(calc_dict,dict) def tearDown(self): - if os.path.exists(os.path.expandvars("$AUTOTST/autotst/calculator/ts")): - shutil.rmtree(os.path.expandvars("$AUTOTST/autotst/calculator/ts")) - if os.path.exists(os.path.expandvars("$AUTOTST/autotst/calculator/species")): - shutil.rmtree(os.path.expandvars("$AUTOTST/autotst/calculator/species")) + ts_dir = project_root() / "autotst" / "calculator" / "ts" + species_dir = project_root() / "autotst" / "calculator" / "species" + if os.path.exists(ts_dir): + shutil.rmtree(ts_dir) + if os.path.exists(species_dir): + shutil.rmtree(species_dir) if __name__ == "__main__": unittest.main(testRunner=unittest.TextTestRunner(verbosity=2)) - \ No newline at end of file + diff --git a/test/bin/g16 b/test/bin/g16 index 3190dcc3..fee539ab 100755 --- a/test/bin/g16 +++ b/test/bin/g16 @@ -1,11 +1,13 @@ #!/usr/bin/env python import os, sys, shutil, time +from pathlib import Path input_file = sys.argv[1].split("/")[-1] + ".log" directory_path = sys.argv[1].strip(input_file) log_file = input_file.replace(".com", ".log") -log_path = os.path.expandvars("$AUTOTST/test/bin/log-files") +project_root = Path(__file__).resolve().parents[2] +log_path = project_root / "test" / "bin" / "log-files" if os.path.join(log_path, log_file): shutil.copy( @@ -13,4 +15,3 @@ if os.path.join(log_path, log_file): os.path.join(directory_path, log_file) ) - From 734c5c13bb23592f09308490be57dd67a61ce41d Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Wed, 8 Apr 2026 01:03:58 +0300 Subject: [PATCH 8/9] Update Makefile to use pytest for unit testing Replace nosetests with pytest and simplify the cleanup logic for temporary test directories. --- Makefile | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index a1ba31e5..edcd4a6e 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,3 @@ unittest unittests test-unittest test-unittests: - nosetests --nocapture --nologcapture --all-modules --verbose --with-coverage --cover-package=autotst - rm -r species - rm -r ts - rm -r test/species - rm -r test/ts + pytest --verbose --tb=short autotst/ + rm -rf species ts test/species test/ts From a49a8559fa0a0e93900addf810adc79024ebc94b Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Wed, 8 Apr 2026 01:04:08 +0300 Subject: [PATCH 9/9] Add GitHub Actions CI workflow for automated testing Configure a continuous integration pipeline to run pytest within a Conda environment on pushes, pull requests to the main branch, and a weekly schedule. --- .github/workflows/ci.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..709e38de --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,33 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: '0 6 * * 1' # Weekly on Monday at 06:00 UTC + +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 15 + defaults: + run: + shell: bash -l {0} + + steps: + - uses: actions/checkout@v4 + + - uses: conda-incubator/setup-miniconda@v3 + with: + environment-file: environment.yml + activate-environment: tst_env + miniforge-version: latest + + - name: Run tests + env: + PYTHONPATH: ${{ github.workspace }} + run: | + pytest --verbose --tb=short autotst/ + rm -rf species ts test/species test/ts