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

Add support for fractional bond orders #222

Open
tovrstra opened this issue Mar 12, 2021 · 1 comment
Open

Add support for fractional bond orders #222

tovrstra opened this issue Mar 12, 2021 · 1 comment
Labels
API breaking Should be done first to stabilize API
Milestone

Comments

@tovrstra
Copy link
Member

At the moment, bond orders are force to be integer because they are stored in the same array containing the atomic indices. This can be fixed easily with a structured data type: np.dtype("int, int, float").

@tovrstra tovrstra added this to the 1.0.0 milestone Mar 24, 2021
@tovrstra
Copy link
Member Author

tovrstra commented Apr 1, 2021

I've added a few PRs adding support for bond information to a few more file formats, just to get a better feel of what is needed, see #248 and #250. There are a few different things needed by various formats. Splitting up the current bonds array into a three attributes seems most appropriate:

  1. bonds just becomes an array with two columns for pairs for atoms (first two columns of the current bonds attribute.
  2. bondtypes is a vector with an integer kind for each bond. Integers can be mapped to various string representations of bond types, to deal with incompatible nomenclatures.
  3. bondorders is a floating point vector with just the bond orders of the bonds listed in bonds. This is needed e.g. by QCSchema.

The usual validation mechanisms can impose consistent sizes of these arrays and a read-only nbond property can be added in line with natom.

In the long run, we can add machinery to deduce a decent bondorders vector from the bondtypes (and vice versa), but that is a secondary concern. (This would make more sense after addressing #191.) Just getting the right attributes in place should be done first because it affects API and blocks a 1.0 release.

@tovrstra tovrstra added the API breaking Should be done first to stabilize API label Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API breaking Should be done first to stabilize API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant