Log packing failures #1067

mattwthompson · 2024-10-02T14:49:23Z

Description

Write error blob to file if Packmol fails (just throw it in working path - simpler than adding a new argument for where to write this to)

Checklist

Add tests
Lint
Update docstrings

codecov · 2024-10-02T14:58:15Z

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.51%. Comparing base (273d27a) to head (d3a2465).
Report is 8 commits behind head on develop.

Additional details and impacted files

mattwthompson · 2024-10-02T15:09:45Z

Here's the UX for a straightforward case, stripping down #1062 a bit

In [1]: from openff.toolkit import Molecule, Topology, Quantity
   ...: from openff.interchange.components._packmol import pack_box, UNIT_CUBE
   ...:
   ...:
   ...: solvent = Molecule.from_smiles('CCCCO')
   ...: solvent.generate_conformers()
   ...:
   ...: ligand = Molecule.from_smiles('CCCCCCCC')
   ...: ligand.generate_conformers()
   ...:
   ...: pack_box(
   ...:     molecules=[solvent],
   ...:     number_of_copies=[100],
   ...:     solute=ligand.to_topology(),
   ...:     tolerance=Quantity("1.0 angstrom"),
   ...:     mass_density=Quantity("100 * gram/milliliter"),
   ...:     box_shape=UNIT_CUBE,
   ...:     center_solute=True,
   ...:     working_directory='.',
   ...:     retain_working_files=False,
   ...: )
   ...:
---------------------------------------------------------------------------
CalledProcessError                        Traceback (most recent call last)
File ~/software/openff-interchange/openff/interchange/components/_packmol.py:705, in pack_box(molecules, number_of_copies, solute, tolerance, box_vectors, mass_density, box_shape, center_solute, working_directory, retain_working_files)
    704 try:
--> 705     result = subprocess.check_output(
    706         packmol_path,
    707         stdin=file_handle,
    708         stderr=subprocess.STDOUT,
    709     )
    710 except subprocess.CalledProcessError as error:
    711     # Custom error codes seem to start at 170
    712     # https://github.com/m3g/packmol/blob/v20.15.1/src/exit_codes.f90#L13-L16

File ~/micromamba/envs/new-models/lib/python3.11/subprocess.py:465, in check_output(timeout, *popenargs, **kwargs)
    463     kwargs['input'] = empty
--> 465 return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
    466            **kwargs).stdout

File ~/micromamba/envs/new-models/lib/python3.11/subprocess.py:569, in run(input, capture_output, timeout, check, *popenargs, **kwargs)
    568     if check and retcode:
--> 569         raise CalledProcessError(retcode, process.args,
    570                                  output=stdout, stderr=stderr)
    571 return CompletedProcess(process.args, retcode, stdout, stderr)

CalledProcessError: Command '/Users/mattthompson/micromamba/envs/new-models/bin/packmol' returned non-zero exit status 173.

The above exception was the direct cause of the following exception:

PACKMOLRuntimeError                       Traceback (most recent call last)
Cell In[1], line 11
      8 ligand = Molecule.from_smiles('CCCCCCCC')
      9 ligand.generate_conformers()
---> 11 pack_box(
     12     molecules=[solvent],
     13     number_of_copies=[100],
     14     solute=ligand.to_topology(),
     15     tolerance=Quantity("1.0 angstrom"),
     16     mass_density=Quantity("100 * gram/milliliter"),
     17     box_shape=UNIT_CUBE,
     18     center_solute=True,
     19     working_directory='.',
     20     retain_working_files=False,
     21 )

File ~/micromamba/envs/new-models/lib/python3.11/site-packages/openff/utilities/utilities.py:80, in requires_package.<locals>.inner_decorator.<locals>.wrapper(*args, **kwargs)
     77 except Exception as e:
     78     raise e
---> 80 return function(*args, **kwargs)

File ~/software/openff-interchange/openff/interchange/components/_packmol.py:715, in pack_box(molecules, number_of_copies, solute, tolerance, box_vectors, mass_density, box_shape, center_solute, working_directory, retain_working_files)
    710     except subprocess.CalledProcessError as error:
    711         # Custom error codes seem to start at 170
    712         # https://github.com/m3g/packmol/blob/v20.15.1/src/exit_codes.f90#L13-L16
    713         open("packmol_error.log", "w").write(error.stdout.decode("utf-8"))
--> 715         raise PACKMOLRuntimeError(
    716             f"PACKMOL failed with error code {error.returncode}. Wrote file packmol_error.log in working "
    717             "directory, which might be a temporary directory. Set the argument `working_directory` to "
    718             "point this to a persistent path.",
    719         ) from error
    721     packmol_succeeded = result.decode("utf-8").find("Success!") > 0
    723 if not packmol_succeeded:

PACKMOLRuntimeError: PACKMOL failed with error code 173. 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.

In [2]: !tail -n 100 packmol_error.log
  Moving worst molecules ...
  Function value before moving molecules:   39.466416204891054
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.466416204786036
  Packing:|0                                                             100%|
          |******************************************************************|
          |******************************************************************|

  Fixing bad orientations ...     35 of     40
  Moving worst molecules ...
  Function value before moving molecules:   39.466416204786036
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.466416204724517
  Packing:|0                                                             100%|
          |******************************************************************|
          |******************************************************************|

  Fixing bad orientations ...     36 of     40
  Moving worst molecules ...
  Function value before moving molecules:   39.466416204724517
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.466416204627130
  Packing:|0                                                             100%|
          |******************************************************************|
          |******************************************************************|

  Fixing bad orientations ...     37 of     40
  Moving worst molecules ...
  Function value before moving molecules:   39.466416204627130
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.466416204593855
  Packing:|0                                                             100%|
          |******************************************************************|
          |******************************************************************|

  Fixing bad orientations ...     38 of     40
  Moving worst molecules ...
  Function value before moving molecules:   39.466416204593855
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.466416204587837
  Packing:|0                                                             100%|
          |******************************************************************|
          |******************************************************************|

  Fixing bad orientations ...     39 of     40
  Moving worst molecules ...
  Function value before moving molecules:   39.466416204587837
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.467380674970428
  Packing:|0                                                             100%|
          |******************************************************************|
          |******************************************************************|
  Fixing bad orientations ...     40 of     40
  Moving worst molecules ...
  Function value before moving molecules:   39.466416265153697
  Type         1 molecules with non-zero contributions:  100.00%
  Moving         5 molecules of type         1
  New positions will be based on good molecules (movebadrandom is not set)
   Moving:|0                                                             100%|
          |******************************************************************|
  Function value after moving molecules:   39.466416258718127

  Restraint-only function value:    39.466416258718127
  Maximum violation of the restraints:   0.18227051192478103
  ERROR: Packmol was unable to put the molecules
         in the desired regions even without
         considering distance tolerances.
         Probably there is something wrong with
         the constraints, since it seems that
         the molecules cannot satisfy them at
         at all.
         Please check the spatial constraints and
         try again.
  >The maximum number of cycles (          40 ) was achieved.
   You may try increasing it with the nloop0 keyword, as in: nloop0 1000
STOP 173

ntBre

LGTM besides the one question about writing a log file instead of writing to stderr. I'm guessing you probably already considered that, though, so happy to approve.

ntBre · 2024-10-03T20:16:25Z

openff/interchange/components/_packmol.py

+                # 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"))


Is there a reason to write this to a file instead of just dumping to stderr? print(error.stdout.decode("utf-8"), file=sys.stderr) would be about as terse and avoid having to warn about a temporary directory down below. Or you could write it into a StringIO and include it in the exception itself.

Mostly that this is the easiest way I could think of dumping the error log in a way that the user could inspect later (but does require opting in to working_directory='.', retain_working_files=False). Also, it can be fairly long, like a few thousand lines for realistic systems. IIUC dumping to STDERR would display the same information to the user but potentially get stuff merged in with other errors (and maybe warnings?) which could be noisier

Sounds good. As a user I think I'd slightly prefer to have it all in one place, but I definitely see the tradeoff and think this is a reasonable choice too!

mattwthompson · 2024-10-03T22:00:12Z

Thanks for the review @ntBre!

mattwthompson changed the base branch from main to develop October 2, 2024 14:49

mattwthompson added 2 commits October 2, 2024 09:51

TST: Test writing log file on convergence failure

61b304d

ENH: Write errors to log file on packing failure

d3a2465

mattwthompson force-pushed the log-packmol-failure branch from 5f00476 to d3a2465 Compare October 2, 2024 14:51

mattwthompson marked this pull request as ready for review October 2, 2024 15:10

ntBre approved these changes Oct 3, 2024

View reviewed changes

mattwthompson merged commit b8b7914 into develop Oct 3, 2024
21 checks passed

mattwthompson mentioned this pull request Oct 3, 2024

Packmol interface: can't pack mnsol benchmark inputs with assumed evaluator defaults #1062

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log packing failures #1067

Log packing failures #1067

mattwthompson commented Oct 2, 2024 •

edited

Loading

codecov bot commented Oct 2, 2024 •

edited

Loading

mattwthompson commented Oct 2, 2024

ntBre left a comment

ntBre Oct 3, 2024

mattwthompson Oct 3, 2024

ntBre Oct 3, 2024

mattwthompson commented Oct 3, 2024

Log packing failures #1067

Log packing failures #1067

Conversation

mattwthompson commented Oct 2, 2024 • edited Loading

Description

Checklist

codecov bot commented Oct 2, 2024 • edited Loading

Codecov Report

mattwthompson commented Oct 2, 2024

ntBre left a comment

Choose a reason for hiding this comment

ntBre Oct 3, 2024

Choose a reason for hiding this comment

mattwthompson Oct 3, 2024

Choose a reason for hiding this comment

ntBre Oct 3, 2024

Choose a reason for hiding this comment

mattwthompson commented Oct 3, 2024

mattwthompson commented Oct 2, 2024 •

edited

Loading

codecov bot commented Oct 2, 2024 •

edited

Loading