Updates for QCFractal 0.54 and OFFTK 0.16 (#277)

* Try testing against QCFractal development * changes suggested by BP * Update releasehistory * try applying @bennybp's suggestion to fix invalid cmiles test * loosen qcfractal constraint and rerun ci * try downpinning offtk * Try installing from offtk fork * unpin offtk * fix yaml formatting * fix yaml formatting * fix protomer enumeration test and clarify behavior * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update protomer tests * update protomer test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Try fixing segfault locally * fix two more instances where _conformers was being set to [] * workaround for another conformers=[]/None scenario, fix typos * update releasehistory * update releasehistory for EnumerateProtomers max_states behavior change * cosmetic commit to kick pre-commit ci * Avoid Python 3.12 with flake8 check --------- Co-authored-by: Jeff Wagner <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
openforcefield · Apr 23, 2024 · fbbbfff · fbbbfff
1 parent 35fe772
commit fbbbfff
Show file tree

Hide file tree

Showing 13 changed files with 72 additions and 18 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -2,12 +2,12 @@ ci:
   autoupdate_schedule: "quarterly"
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.5.0
+  rev: v4.6.0
   hooks:
   - id: check-yaml
   - id: debug-statements
 - repo: https://github.com/psf/black
-  rev: 24.3.0
+  rev: 24.4.0
   hooks:
   - id: black
     files: ^openff
@@ -32,3 +32,5 @@ repos:
     - id: nbqa-flake8
       args:
         - '--select=F'
+default_language_version:
+  python: python3.11
diff --git a/devtools/conda-envs/basic.yaml b/devtools/conda-envs/basic.yaml
@@ -21,7 +21,7 @@ dependencies:
 
   - qcengine >=0.25
   - qcelemental >=0.25.1
-  - qcfractal =0.53
+  - qcfractal >=0.54
   - qcarchivetesting
   - qcportal
 

diff --git a/devtools/conda-envs/psi4.yaml b/devtools/conda-envs/psi4.yaml
@@ -19,7 +19,7 @@ dependencies:
 
   - qcengine >=0.25
   - qcelemental >=0.25.1
-  - qcfractal =0.53
+  - qcfractal >=0.54
   - qcarchivetesting
   - qcportal
 

diff --git a/docs/releasehistory.md b/docs/releasehistory.md
@@ -9,7 +9,19 @@ Releases follow the ``major.minor.micro`` scheme recommended by
 
 Releases are given with dates in DD-MM-YYYY format.
 
-## Current development
+<!--## Version / Date DD-MM-YYYY -->
+## 0.51.0 / 23-04-2024
+
+### Behaviors changed
+
+* [PR #277:] Changes the behavior of the `max_states` named argument to `workflow_components.EnumerateProtomers`. Previously this could return anywhere from `1` to `max_states+2`, but now it can return `1` to `max_states+1` (depending on whether the backend includes the input in the protomers that are generated).  
+
+### Bugfixes
+
+* [PR #277:] Updates for QCPortal 0.54 (#275) and OpenFF Toolkit 0.16 (#278) [@bennybp @mattwthompson @j-wags]
+
+
+## 0.50.3 / 24-03-2024
 
 ### Bugfixes
 
@@ -21,7 +33,6 @@ Releases are given with dates in DD-MM-YYYY format.
 
 * [PR #270:] Speed up `TorsionDriveResultCollection.to_records` by batching requests [@ntBre]
 
-<!--## Version / Date DD-MM-YYYY -->
 ## 0.50.2 / 24-01-2024
 
 ### New Features
@@ -111,6 +122,7 @@ For more information on this release, see https://github.com/openforcefield/open
 [PR #260:]: https://github.com/openforcefield/openff-qcsubmit/pull/260
 [PR #268:]: https://github.com/openforcefield/openff-qcsubmit/pull/268
 [PR #270:]: https://github.com/openforcefield/openff-qcsubmit/pull/270
+[PR #277:]: https://github.com/openforcefield/openff-qcsubmit/pull/277
 
 [@jthorton]: https://github.com/jthorton
 [@dotsdl]: https://github.com/dotsdl

diff --git a/openff/qcsubmit/_tests/results/__init__.py b/openff/qcsubmit/_tests/results/__init__.py
@@ -180,7 +180,7 @@ def mock_torsion_drive_result_collection(
                             ),
                         ),
                     ),
-                    initial_molecules_ids_=[
+                    initial_molecules_ids=[
                         i + 1
                         for i in range(
                             molecules[address][int(entry.record_id) - 1].n_conformers

diff --git a/openff/qcsubmit/_tests/results/test_results.py b/openff/qcsubmit/_tests/results/test_results.py
@@ -406,7 +406,7 @@ def test_torsion_smirnoff_coverage(public_client, monkeypatch):
                         ),
                     ),
                     id=entry.record_id,
-                    initial_molecules_=[],
+                    initial_molecules=[],
                     status=RecordStatusEnum.complete,
                     is_service=False,
                     created_on=datetime.datetime(2022, 4, 21, 0, 0, 0),

diff --git a/openff/qcsubmit/_tests/test_datasets.py b/openff/qcsubmit/_tests/test_datasets.py
@@ -174,7 +174,7 @@ def test_componentresult_str():
     )
 
 
-def test_componetresult_deduplication_standard():
+def test_componentresult_deduplication_standard():
     """
     Test the components results ability to deduplicate molecules.
     """
@@ -202,7 +202,7 @@ def test_componetresult_deduplication_standard():
     assert len(result.filtered) == 0
 
 
-def test_componetresult_directory():
+def test_componentresult_directory():
     """
     Test loading up some molecules from a directory of files.
     """
@@ -233,7 +233,7 @@ def test_componetresult_directory():
         pytest.param("hdf5-example.hdf5", id="HDF5 file"),
     ],
 )
-def test_componetresult_input_file(file_name):
+def test_componentresult_input_file(file_name):
     """
     Test loading up some molecules from an input file
     """

diff --git a/openff/qcsubmit/_tests/test_submissions.py b/openff/qcsubmit/_tests/test_submissions.py
@@ -1424,6 +1424,7 @@ def test_invalid_cmiles(fulltest_client, factory_type, result_collection_type):
     entries[0].attributes[
         "canonical_isomeric_explicit_hydrogen_mapped_smiles"
     ] = "[H:4][C:2](=[O:1])[OH:3]"
+    ds._cache_data.update_entries(entries)
     results = result_collection_type.from_datasets(datasets=ds)
     assert results.n_molecules == 1
     with pytest.warns(UserWarning, match="invalid CMILES"):

diff --git a/openff/qcsubmit/_tests/test_workflow_components.py b/openff/qcsubmit/_tests/test_workflow_components.py
@@ -642,7 +642,40 @@ def test_enumerating_protomers_apply():
 
     assert mol in result.molecules
     # this means that the parent molecule was included
-    assert result.n_molecules == 3
+    assert result.n_molecules == 2
+
+    # Test that the input is always in the output, even when it
+    # wouldn't have been generated as a possible protomer
+    enumerate_protomers = workflow_components.EnumerateProtomers(max_states=1)
+    weird_mol = Molecule.from_smiles("[N-]([H])[H]")
+
+    result = enumerate_protomers.apply(
+        [
+            weird_mol,
+        ],
+        processors=1,
+        toolkit_registry=GLOBAL_TOOLKIT_REGISTRY,
+    )
+
+    assert weird_mol in result.molecules
+    # this means that the parent molecule was included
+    assert result.n_molecules == 2
+
+    # Test that the deduplication works (this molecule has exactly 4 protomers,
+    # so asking for up to 5 states should yield 4)
+    enumerate_protomers = workflow_components.EnumerateProtomers(max_states=5)
+    mol = Molecule.from_smiles("Oc2ccc(c1ccncc1)cc2")
+    result = enumerate_protomers.apply(
+        [
+            mol,
+        ],
+        processors=1,
+        toolkit_registry=GLOBAL_TOOLKIT_REGISTRY,
+    )
+
+    assert mol in result.molecules
+    # this means that the parent molecule was included
+    assert result.n_molecules == 4
 
 
 def test_coverage_filter_remove():

diff --git a/openff/qcsubmit/utils/smirnoff.py b/openff/qcsubmit/utils/smirnoff.py
@@ -206,7 +206,7 @@ def combine_openff_molecules(molecules: List[Molecule]) -> Molecule:
 
     master_mol = copy.deepcopy(molecules.pop(0))
     conformers = [*master_mol.conformers]
-    master_mol._conformers = []
+    master_mol._conformers = None
     index_map = {}
     for molecule in molecules:
         for atom in molecule.atoms:

diff --git a/openff/qcsubmit/utils/visualize.py b/openff/qcsubmit/utils/visualize.py
@@ -75,7 +75,7 @@ def _create_openeye_pdf(molecules: List[Molecule], file_name: str, columns: int)
     # now we load the molecules
     for off_mol in molecules:
         off_mol = copy.deepcopy(off_mol)
-        off_mol._conformers = []
+        off_mol._conformers = None
         off_mol.name = None
 
         cell = report.NewCell()

diff --git a/openff/qcsubmit/workflow_components/state_enumeration.py b/openff/qcsubmit/workflow_components/state_enumeration.py
@@ -203,7 +203,7 @@ def _apply(
         self, molecules: List[Molecule], toolkit_registry: ToolkitRegistry
     ) -> ComponentResult:
         """
-        Enumerate the formal charges of the molecule.
+        Enumerate the protonation states of the molecule.
 
         Parameters:
             molecules: The list of molecules the component should be applied on.
@@ -212,6 +212,9 @@ def _apply(
         Returns:
             A [ComponentResult][qcsubmit.datasets.ComponentResult] instance containing information about the molecules
             that passed and were filtered by the component and details about the component which generated the result.
+            Note that the input molecule is guaranteed to be included in this output, which in some cases may cause the
+            number of molecules in the result to be one greater than max_states, or may cause a molecule in the results
+            to have multiple conformers.
 
         Important:
             This is only possible using Openeye so far, if openeye is not available this step will fail.

diff --git a/openff/qcsubmit/workflow_components/utils.py b/openff/qcsubmit/workflow_components/utils.py
@@ -541,7 +541,7 @@ def n_impropers(self) -> int:
 
 class ComponentResult:
     """
-    Class to contain molecules after the execution of a workflow component this automatically applies de-duplication to
+    Class to contain molecules after the execution of a workflow component. This automatically applies de-duplication to
     the molecules. For example if a molecule is already in the molecules list it will not be added but any conformers
     will be kept and transferred.
 
@@ -760,7 +760,10 @@ def add_molecule(self, molecule: off.Molecule) -> bool:
                     new_conf = unit.Quantity(new_conformer, unit.angstrom)
 
                     # check if the conformer is already on the molecule
-                    for old_conformer in self._molecules[molecule_hash].conformers:
+                    old_conformers = self._molecules[molecule_hash].conformers
+                    if old_conformers is None:
+                        old_conformers = []
+                    for old_conformer in old_conformers:
                         if old_conformer.tolist() == new_conf.tolist():
                             break
                     else:
@@ -772,7 +775,7 @@ def add_molecule(self, molecule: off.Molecule) -> bool:
         else:
             if molecule.n_conformers == 0:
                 # make sure this is a list to avoid errors
-                molecule._conformers = []
+                molecule._conformers = None
             self._molecules[molecule_hash] = molecule
             return False