Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parametrization tests failing #316

Open
ijpulidos opened this issue Nov 29, 2023 · 3 comments
Open

Parametrization tests failing #316

ijpulidos opened this issue Nov 29, 2023 · 3 comments

Comments

@ijpulidos
Copy link
Collaborator

          I'm seeing this error:
FAILED openmmforcefields/tests/test_template_generators.py::TestSMIRNOFFTemplateGenerator::test_partial_charges_are_none - RuntimeError: C=O could not be fully assigned charges. Charges were assigned to atoms set() but the molecule contains {0, 1, 2, 3}.
FAILED openmmforcefields/tests/test_template_generators.py::TestSMIRNOFFTemplateGenerator::test_parameterize - RuntimeError: C=O could not be fully assigned charges. Charges were assigned to atoms set() but the molecule contains {0, 1, 2, 3}.

Which I've seen many times before but keep forgetting the source of

Originally posted by @mattwthompson in #313 (comment)

@mattwthompson
Copy link
Collaborator

The last time this happened it appeared to be because the OpenEye license was out of date, although that doesn't seem likely since this line passes?

python -c "from openeye import oechem; assert oechem.OEChemIsLicensed()"

Related #278 #279 #277

@mikemhenry
Copy link
Collaborator

Hmmm yes this smells like something is failing deeper in the codebase but the issue isn't being bubbled up and instead we end up with a None or something

@mattwthompson
Copy link
Collaborator

The error message isn't particularly helpful since it captures atoms missing charges in the context of trying a bunch of different things, whereas only one attempt (AM1-BCC) is really relevant here. So (guessing) the other charge methods (library charges, charge increments, charge_from_molecules) do nothing and when AM1-BCC fails, it is treated like any other failure and just ticks on to the next method. But AM1-BCC is the last one, so there's nothing to fall back to.

Here's where it's thrown: https://github.com/openforcefield/openff-interchange/blob/v0.3.18/openff/interchange/smirnoff/_nonbonded.py#L762C9-L769

I think the traceback could be more helpful to downstream developers (like me!) including information such as what was tried and what failed, or maybe if AM1-BCC fails there should be a more specific exception there instead of later, since AM1-BCC failing will cause the failure later. If I'm right, I'll patch Interchange to be more helpful here, but that wouldn't completely resolve the issue of these simple molecules not getting charges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants