ZimmermanGroup / pyGSM

Thermal and photochemical reaction path optimization and discovery
MIT License
50 stars 26 forks source link

Cryptic error message when fragments in xyz file have intermixed indices #47

Open joshkamm opened 1 year ago

joshkamm commented 1 year ago

It seems like when you have multiple disconnected molecular components in an input xyz file where the indices of one fragment fall in the middle of indices of another fragment, the code can fail at pygsm/coordinate_systems/primitive_internals.py line 1343 with a generic RuntimeError and it's not immediately clear why. Both @epunzal2 and @SoumiKDaS1701 have run into this and one scenario that causes intermixed indices is when a catalyst has multiple ligands that bind and dissociate over the course of a reaction mechanism.

Ideally whether fragments fall into disjoint ranges of indices wouldn't impact the code, but if that would be difficult a clearer error message could at least alert a user to how to work around the issue. @craldaz do you have a sense of whether this would show up in a lot of places in the code?

craldaz commented 1 year ago

I think the problem is that the Translation and Rotation internal coordinates expect all indices of fragments to be contiguous. Otherwise it isn't easy to do the block diagonal stuff. The error message can certainly be more clear and there should also be the option to ignore block diagonalization. One option is to try the regular delocalized internal coordinates. However, it may also fail with the same message -- not sure! Maybe send over the initial.xyz and I can give it a shot to see what's going on. Happy to try to add any suggestions also.

joshkamm commented 1 year ago

Sorry for the delay. I finally got around to at least adding a more specific error message to the exception that I was getting. I tried to create an isolated environment for testing that's separate from the library I'm building that uses pyGSM but I wasn't quickly able to come up with a simple example that triggers the exception. I can try to use the example that originally broke it but I'll have to merge the two environments to do that.