diff --git a/openff/interchange/_tests/unit_tests/smirnoff/test_nonbonded.py b/openff/interchange/_tests/unit_tests/smirnoff/test_nonbonded.py index fe3af8bf1..c39477f40 100644 --- a/openff/interchange/_tests/unit_tests/smirnoff/test_nonbonded.py +++ b/openff/interchange/_tests/unit_tests/smirnoff/test_nonbonded.py @@ -160,6 +160,30 @@ def test_nonstandard_scale_factors( for index, factor in factors.items(): assert getattr(interchange[collection], f"scale1{index}") == factor + @pytest.mark.skip("Behavior not implemented yet") + def test_nagl_charges_fails_fallback(self, sage_with_nagl_charges): + """Test whether molecules that fail having charges assigned by NAGLCharges successfully + fall back to lower-precedence charge methods + """ + # Create a molecule that NAGL can't charge + mol = Molecule.from_smiles("B") + # Create a FF with a ChargeIncrementModel that CAN charge the molecule + sage_with_nagl_charges.deregister_parameter_handler("Bonds") + sage_with_nagl_charges.deregister_parameter_handler("Constraints") + sage_with_nagl_charges.deregister_parameter_handler("Angles") + sage_with_nagl_charges.deregister_parameter_handler("ProperTorsions") + sage_with_nagl_charges.deregister_parameter_handler("ImproperTorsions") + sage_with_nagl_charges.deregister_parameter_handler("vdW") + sage_with_nagl_charges.get_parameter_handler( + "ChargeIncrementModel", + { + "partial_charge_method": "formal_charge", + "version": "0.3", + }, + ) + # Ensure that no error is raised when assigning charges, since NAGL will fail but CIMH will succeed + sage_with_nagl_charges.create_interchange(mol.to_topology()) + class TestvdWUpDownConversion: def test_upconversion(self): diff --git a/openff/interchange/smirnoff/_nonbonded.py b/openff/interchange/smirnoff/_nonbonded.py index e97d5f55e..1b32d9496 100644 --- a/openff/interchange/smirnoff/_nonbonded.py +++ b/openff/interchange/smirnoff/_nonbonded.py @@ -744,7 +744,7 @@ def _find_reference_matches( potentials: dict[PotentialKey, Potential] = dict() expected_matches = {i for i in range(unique_molecule.n_atoms)} - + exceptions = [] for handler_type in cls.parameter_handler_precedence(): if handler_type not in parameter_handlers: continue @@ -761,15 +761,19 @@ def _find_reference_matches( unique_molecule, ) - if handler_type in ["ToolkitAM1BCC", "ChargeIncrementModel"]: - ( - partial_charge_method, - am1_matches, - am1_potentials, - ) = cls._find_charge_model_matches( - parameter_handler, - unique_molecule, - ) + if handler_type in ["ToolkitAM1BCC", "ChargeIncrementModel", "NAGLCharges"]: + try: + ( + partial_charge_method, + am1_matches, + am1_potentials, + ) = cls._find_charge_model_matches( + parameter_handler, + unique_molecule, + ) + except ValueError as e: + exceptions.append(e) + continue if slot_matches is None and am1_matches is None: raise NotImplementedError() @@ -813,10 +817,13 @@ def _find_reference_matches( found_matches = {index for key in matches for index in key.atom_indices} if found_matches != expected_matches: + exceptions_txt = "\n\n\n".join([str(i) for i in exceptions]) + raise RuntimeError( f"{unique_molecule.to_smiles(explicit_hydrogens=False)} could " "not be fully assigned charges. Charges were assigned to atoms " - f"{found_matches} but the molecule contains {expected_matches}.", + f"{found_matches} but the molecule contains {expected_matches}." + f"\n\n{exceptions_txt}", ) return matches, potentials