openforcefield / openff-toolkit

The Open Forcefield Toolkit provides implementations of the SMIRNOFF format, parameterization engine, and other tools. Documentation available at http://open-forcefield-toolkit.readthedocs.io
http://openforcefield.org
MIT License
305 stars 90 forks source link

Port topology notebook to PDB cookbook #1732

Closed mattwthompson closed 7 months ago

mattwthompson commented 9 months ago

Resolves #1692

review-notebook-app[bot] commented 9 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov[bot] commented 9 months ago

Codecov Report

Merging #1732 (1d4711a) into main (779565b) will decrease coverage by 0.02%. Report is 4 commits behind head on main. The diff coverage is n/a.

:exclamation: Current head 1d4711a differs from pull request most recent head afb93c4. Consider uploading reports for the commit afb93c4 to get more accurate results

Additional details and impacted files
j-wags commented 9 months ago

One of the big points about this notebook is that these PDB files are actually taken from and AMBER and GROMACS tutorials. There were a few tweaks that I had to made to make them loadable, and the PDB files have somewhat generous solvent boxes that are so large they'll gum up our CI/docs rendering. So on my to-do list is:

j-wags commented 9 months ago

Turning this over to @Yoshanuikabundi - Two things that would be good are:

As a stretch goal, If we REALLY wanted this to showcase interoperability, we could include the commands from the GROMACS and AMBER tutorials that made these files, but there were a few messy steps (like deleting a H from an N terminus and using RDKit to add CONECT for a ligand) that were a bit messy. So this may be more confusing than it's worth.

mattwthompson commented 9 months ago

I'm personally -1 on fully documenting the process by which the data could be re-generated here. I don't think that's what we want to show off; in other words, if something changes with the tutorials I don't see a reason we should update this cookbook.

Yoshanuikabundi commented 9 months ago

@mattwthompson - for future reference, to get the widgets to display in Sphinx you just need to save the widget state in the notebook :smile: In modern versions of Notebook and Lab this is a tick box in Settings -> Save Widget State Automatically, and I think it might even notice that widget state was saved previously and tick this automatically. In older versions of notebook it was under the Widgets menu IIRC and you had to click it every time you saved the notebook.

This basically just writes the PDB file being send to NGLView in the notebook, and there's JS automatically embedded by Sphinx that loads it and runs the widgets. The catch is that there's no Python runtime in the browser, so only the stuff handled by JS is dynamic - for example, our NGLView trajectory handler passes one frame at a time from Python to NGLView, so the scan bar and play button show up but the frame never changes.

mattwthompson commented 9 months ago

@Yoshanuikabundi could I convince you to capture that somewhere like the developer docs? This is valuable (and not-very-transient) information

Yoshanuikabundi commented 7 months ago

Hey @mattwthompson, is there more you wanted to do on this? I'm going over it now but just wanted to make sure I didn't merge it without missing something you wanted.

Also

a cubic solvent box is used instead of an octohedron

Does the Toolkit not support octohedra? I thought we could handle arbitrary triclinic boxes. I can rephrase to still state it is cubic without implying that we don't support octohedra :)

j-wags commented 7 months ago

I don't think that all parts of our codebase support truncated octahedrons. Interchange has some testing for exporting different sorts of boxes to GROMACS, but I don't recall adding any particular support in the Toolkit. Particularly, I don't think we support the vanilla "load PDB --> assign params --> simulate in OpenMM" workflow for octahedrons.

I'll revert that bit and merge. Thanks all!