openforcefield / openff-qcsubmit

Automated tools for submitting molecules to QCFractal
https://openff-qcsubmit.readthedocs.io/en/latest/index.html
MIT License
26 stars 4 forks source link

`cached_query_torsion_drive_results` fails for multi-dimensional grid ids #201

Closed chapincavender closed 1 year ago

chapincavender commented 2 years ago

If a TorsionDriveResultCollection contains TorsionDrives with multi-dimensional grid indices, cached_query_torsion_drive_results will fail here with ValueError: could not convert string to float.

The TorsionDrive grid indices are stored as stringified tuples such as '(-165)'. For multi-dimensional TorsionDrives, the grid indices look like '(-165, -165)' so that float(x[1:-1]) fails. I think something like grid_ids.sort(key=lambda s: tuple(float(x) for x in s[1:-1].split(", "))) will work for both one-dimensional and multi-dimensional grid indices.

I have this patch in a new branch that I'm testing now. Any ideas for a solution that handles this problem more elegantly?

dotsdl commented 2 years ago

Thanks @chapincavender! Can you make a PR with your change? I think your approach is close to what I might do, but I'd probably go with:

grid_ids.sort(key=lambda s: tuple(float(x) for x in s.strip('()').split(',')))

This avoids any assumptions on slicing.

chapincavender commented 2 years ago

Fixed in #202. It looks like the grid indices are stringified lists instead of stringified tuples as I stated above, so I went with split("[]") instead of splicing.

mattwthompson commented 1 year ago

Closed via #202 and released in version 0.3.2