Make PACKMOL wrappers public

[mattwthompson]

Description

Resolves #1170

Checklist

• Add tests
• Lint
• Update docstrings

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov]

Codecov Report

Attention: Patch coverage is 91.89189% with 21 lines in your changes missing coverage. Please review.

Project coverage is 93.85%. Comparing base (56af960) to head (2301d88).

Files with missing lines	Patch %	Lines
openff/interchange/packmol.py	91.73%	21 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1252      +/-   ##
==========================================
+ Coverage   93.57%   93.85%   +0.27%     
==========================================
  Files          72       73       +1     
  Lines        6228     6234       +6     
==========================================
+ Hits         5828     5851      +23     
+ Misses        400      383      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mattwthompson]

No idea why the docs aren't building ... works for me locally with a fresh environment 😕

[mattwthompson]

242 | WARNING: don't know which module to import for autodocumenting 'UNIT_CUBE' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
243 | WARNING: don't know which module to import for autodocumenting 'RHOMBIC_DODECAHEDRON' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
244 | WARNING: don't know which module to import for autodocumenting 'RHOMBIC_DODECAHEDRON_XYHEX' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)

[j-wags]

I haven't looked at the docs (sorry, maybe something @Yoshanuikabundi would be more efficient at) but once those build get fixed this PR looks good to me!

[Yoshanuikabundi]

I have serious misgivings about the way we currently add a buffer around the packmol box, and then try to remove it by growing the box to account for it. I'd like to discuss the issue synchronously before merging please!

Edit: but I did fix the docs issue!

[Yoshanuikabundi]

I think this line has been the source of much confusion in the packmol wrapper. The notes on the public functions state:

When calling Packmol, each linear dimension of the box is scaled up by 10%. However, Packmol by default adds a small buffer (defined by the tolerance argument which otherwise defines the minimum distance, default 2 Angstrom) at the end of the packed box, which causes small voids when tiling copies of each periodic image. This void is removed in hopes of faster equilibration times but the box density is slightly increased as a result.

That small buffer isn't added by packmol, it's added by me, here. I expected that at high (eg, biological) densities you might get clashes because packmol doesn't account for PBCs, and so I added a small, tunable buffer region. This box size refers to the size of the box that packmol fills with solvent, not the simulation box (which packmol has no knowledge of and is set on the topology itself). A lot of the complex code for handling PBCs in this module is to get everything into a single rectangular prism so that packmol can just treat it as a box. This allows the packmol wrapper to be used for both NVT and NVP simulations in arbitrary triclinic boxes without the user having any surprises, and in my experience the time taken to fill the box in NVT is very small (though I admit full equilibration might take longer).

If this buffer region isn't needed or desired, I would strongly prefer we remove it and remove the code enlarging the box by 10% than leave things as they are. This buffer region doesn't disrupt any of the users expectations - Packmol boxes are expected to need thorough equilibration anyway.

If this doesn't make sense or @mattwthompson disagrees, I would like to ask that we delay merging this until we can discuss it synchronously please!

[mattwthompson]

I am splitting this change off into a separate PR