Proposal for periodic boundary conditions

[bbucior]

See nicely formatted version of text at https://github.com/bbucior/enhancement-proposals/blob/periodic-boundaries/periodic-boundaries/periodic-boundaries.md

There are many details to finalize before making API changes, so I would appreciate any feedback.

[ghutchis]

Thanks for the writeup. I think this is really needed, so I appreciate the thought involved.

I think one way of handling the "stub" problem is to work on a method, similar to CCDC Mercury for "molecule wrapping" - that is, given the atoms in the asymmetric unit, find the positions that minimize bond lengths. In general, that happens by default, but not always.

[ghutchis]

I'll look at this in more detail later in the week when I have some time. (It's NSF deadline season.)

[ghutchis]

I think I'm going to rule out adding OBMol::GetPeriodicLattice because it's un-needed most of the current code (including clients) use the unit cell directly.

I'm OK adding a flag, because this doesn't add additional API calls, but does make it easier to check if an OBMol is periodic without having to check if OBUnitCell is set. That seems like a clear win.

Eventually having an OBCoord class seems like a win, since it could simplify some types of conversion:

• Fractional to Fractional (ie., nothing will change with the coords)
• Z-matrix to Z-matrix (same thing - nothing converted).
  .. etc.

Let's have that as a future goal.

Otherwise, I think the proposal is good and I look forward to seeing patches. :-)

[bbucior]

Thank you for the suggestions! I'll take out the GetPeriodicLattice code (the second copy of the unit cell also led to some inconsistency issues recently). One of my projects needed a "molecule wrapping" feature similar to what you described, so I may be able to refactor that code. I have several deadlines coming up this month, but I hope to get the patches ready (with tests) around mid-November. I'm excited to get this merged!

[bbucior]

FYI this will likely get pushed back until late December or January. I have to wrap up another project first.

[ghutchis]

(bump) - this would be really cool. @bbucior - is your time opening up any this spring?

More importantly, is this something that can be done in small pieces?

[bbucior]

Thanks for following up with me. My schedule should be opening up in about a month: the other project is written up and just needs another round of revisions. Then getting the PBC code merged to OB will become my research bottleneck, so that will ensure it gets completed. If research priorities change again for some reason, I'll carve out some dedicated time for this in late April. Will that work with OB 3.0 plans?

It looks like there's three main steps before it's ready for PR:

1. Removing the extra copy of OBUnitCell and related convenience functions (GetPeriodicLattice, etc.), which aren't needed in the API
2. Taking another read through the diff and TODOs on the Github branch and my research code to see if there's anything major I missed.
3. Writing test cases

I think other features like SVG drawings, "molecule wrapping," and warning stubs for unsupported formats can be deferred to later PRs, especially since periodicity is manually enabled by the user.

Do you have any ideas for good test cases? I've thought of things like checking the accuracy of a known bond length, angle, and torsion across periodic boundaries. Possibly also reading/building a molecule then exporting a CIF with periodic bonds and the corresponding SMILES. Any tips? I'm struggling to figure out what and how much is best for maintainability.