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.

Sign up for GitHub

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

Jump to bottom

Setup qcel.models.Molecule with non-contiguous fragments #298

Open
awvwgk opened this issue Nov 8, 2022 · 5 comments · May be fixed by #299
Open

Setup qcel.models.Molecule with non-contiguous fragments #298

awvwgk opened this issue Nov 8, 2022 · 5 comments · May be fixed by #299

Comments

@awvwgk
Copy link
Contributor

awvwgk commented Nov 8, 2022 •
edited

Trying the following example molecule from S22, I notice that there is something odd happening for molecules with fragment information:

import qcelemental as qcel

data = {
    "symbols": [
        "N", "N", "H", "H", "H", "H", "H", "H"
    ],
    "geometry": [
        -2.9833455085754399E+0, -8.8082052767279997E-2,  0.0000000000000000E+0,
         2.9833455085754399E+0,  8.8082052767279997E-2,  0.0000000000000000E+0,
        -4.0792036056518599E+0,  2.5775116682053001E-1,  1.5298565626144400E+0,
        -1.6052680015564000E+0,  1.2438048124313399E+0,  0.0000000000000000E+0,
        -4.0792036056518599E+0,  2.5775116682053001E-1, -1.5298565626144400E+0,
         4.0792036056518599E+0, -2.5775116682053001E-1, -1.5298565626144400E+0,
         1.6052680015564000E+0, -1.2438048124313399E+0,  0.0000000000000000E+0,
         4.0792036056518599E+0, -2.5775116682053001E-1,  1.5298565626144400E+0
    ],
    "fragments": [
        [0, 2, 3, 4],
        [1, 5, 6, 7],
    ],
}

for validate in (True, False):
    try:
        mol = qcel.models.Molecule(**data, validate=validate)
    except qcel.exceptions.ValidationError as e:
        print(f"Fails to setup with validate={validate}\n{str(e)}")

Output from the run is an unexpected validation error when validating the input

Warning: QCElemental is reordering atoms to accommodate non-contiguous fragments
Fails to setup with validate=True
Error: QCElemental would need to reorder atoms to accommodate non-contiguous fragments

Why are non-contiguous fragments a validation error in qcel? The issue is probably that throw_reorder is always true at:

QCElemental/qcelemental/molparse/from_schema.py

Line 57 in 292350f

throw_reorder=True,

while the default is false at

QCElemental/qcelemental/molparse/from_schema.py

Line 103 in 292350f

throw_reorder: bool = False,

but there seems to be no way to toggle throw_reorder other than skipping the validation step.
@loriab
Copy link
Collaborator

loriab commented Nov 11, 2022

Ah yes, in the beginning it was requested that fragments be allowed to be noncontiguous. So the good news is that I wrote the hard part (molparse validator) to accommodate. Then we ran into there being zero downstream programs that could actually work with noncontig. I can probably expose throw_reorder to the pydantic model, but results should be checked carefully at first. So you've got a downstream that can use noncontig? I can see how dispersion on fragments would be a good case, esp. as geometry is fixed.

@awvwgk
Copy link
Contributor Author

awvwgk commented Nov 11, 2022

Thanks for the context, yes I tried testing non-contiguous fragments for a workflow which can work with those robustly, but directly hit a hard-stop. Now I work around by handling the fragment information outside of the qcel models because qcel is still more convenient than rolling my own.

@loriab
Copy link
Collaborator

loriab commented Nov 11, 2022

Ok, I looked a bit more, and it's the molparse internal representation (aka psi's) that works in fragment separators (requires contiguous) rather than fragment lists. Thus, molparse always contiguizes, and throw_reorder merely toggles between let-it-reorder and stop-in-tracks.

So, an allow_noncontiguous_fragments_but_reorder=True is not too hard (just lifts the throw), but it's not keeping with good qcschema practice (changing up the input for convenience), and it's probably not what your workflow wants, is it?. A proper handling of noncontig would need more extensive renovation.

Is this xtb that can work with non-contig or a workflow w/o a program in particular? For now, your workaround is probably best.

@awvwgk
Copy link
Contributor Author

awvwgk commented Nov 11, 2022

I could live with qcel reordering the atoms, just leaving them in their order would be preferred, of course.

Generally, the reordering seems like something psi4 should apply as needed, similar as we handle ghost atoms in DFT-D4, qcel / qcng could provide a convenience function for permuting atoms (together with all associated properties).

@loriab loriab linked a pull request Nov 11, 2022 that will close this issue
Open
2 tasks
@loriab
Copy link
Collaborator

loriab commented Nov 13, 2022

There's a PR up @awvwgk , just in case you didn't get pinged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

noncontig passthrough loriab/QCElemental
2 participants
@loriab @awvwgk