From 61b304d2c71ca3ccdbdd405a3287d709c047f2ac Mon Sep 17 00:00:00 2001 From: "Matthew W. Thompson" Date: Wed, 2 Oct 2024 09:48:39 -0500 Subject: [PATCH 1/2] TST: Test writing log file on convergence failure --- .../unit_tests/components/test_packmol.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/openff/interchange/_tests/unit_tests/components/test_packmol.py b/openff/interchange/_tests/unit_tests/components/test_packmol.py index b77480a5..362a6fbd 100644 --- a/openff/interchange/_tests/unit_tests/components/test_packmol.py +++ b/openff/interchange/_tests/unit_tests/components/test_packmol.py @@ -2,6 +2,8 @@ Units tests for openff.interchange.components._packmol """ +import pathlib + import numpy import pytest from openff.toolkit.topology import Molecule @@ -422,3 +424,22 @@ def test_load_100_000_atoms(self): tolerance=1.0 * unit.angstrom, mass_density=0.1 * unit.grams / unit.milliliters, ) + + @pytest.mark.parametrize("use_local_path", [False, True]) + def test_save_error_on_convergence_failure(self, use_local_path): + with pytest.raises( + PACKMOLRuntimeError, + match="failed with error code 173", + ): + pack_box( + molecules=[Molecule.from_smiles("CCO")], + number_of_copies=[100], + box_shape=UNIT_CUBE, + mass_density=100 * unit.grams / unit.milliliters, + working_directory="." if use_local_path else None, + ) + + if use_local_path: + assert "STOP 173" in open("packmol_error.log").read() + else: + assert not pathlib.Path("packmol_error.log").is_file() From d3a24659b46ad7a72731d9710afc2ad9b1a05b44 Mon Sep 17 00:00:00 2001 From: "Matthew W. Thompson" Date: Wed, 2 Oct 2024 09:50:43 -0500 Subject: [PATCH 2/2] ENH: Write errors to log file on packing failure --- openff/interchange/components/_packmol.py | 24 ++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/openff/interchange/components/_packmol.py b/openff/interchange/components/_packmol.py index 8843dc15..fde1125e 100644 --- a/openff/interchange/components/_packmol.py +++ b/openff/interchange/components/_packmol.py @@ -400,7 +400,7 @@ def _create_solute_pdb( ), ) # Write to pdb - solute_pdb_filename = "solvate.pdb" + solute_pdb_filename = "_PACKING_SOLUTE.pdb" topology.to_file( solute_pdb_filename, file_format="PDB", @@ -425,7 +425,7 @@ def _create_molecule_pdbs(molecules: list[Molecule]) -> list[str]: # right order, we'll be able to read packmol's output, since we # just load the PDB coordinates into a topology created from its # component molecules. - pdb_file_name = f"{index}.pdb" + pdb_file_name = f"_PACKING_MOLECULE{index}.pdb" pdb_file_names.append(pdb_file_name) molecule.to_file( @@ -706,15 +706,25 @@ def pack_box( packmol_path, stdin=file_handle, stderr=subprocess.STDOUT, - ).decode("utf-8") + ) except subprocess.CalledProcessError as error: - if "173" in str(error): - raise PACKMOLRuntimeError from error + # Custom error codes seem to start at 170 + # https://github.com/m3g/packmol/blob/v20.15.1/src/exit_codes.f90#L13-L16 + open("packmol_error.log", "w").write(error.stdout.decode("utf-8")) - packmol_succeeded = result.find("Success!") > 0 + raise PACKMOLRuntimeError( + f"PACKMOL failed with error code {error.returncode}. Wrote file packmol_error.log in working " + "directory, which might be a temporary directory. Set the argument `working_directory` to " + "point this to a persistent path.", + ) from error + + packmol_succeeded = result.decode("utf-8").find("Success!") > 0 if not packmol_succeeded: - raise PACKMOLRuntimeError(result) + raise PACKMOLRuntimeError( + "PACKMOL did not raise an error code, but 'Success!' not found in output. " + "Please raise an issue showing how you arrived at this error.", + ) positions = _load_positions(output_file_path)