update for new cinnabar API#311
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
# Conflicts: # environment.yaml
| @@ -5,69 +5,29 @@ | |||
| "id": "ddae61fd-fa15-4be9-9160-ba3e195c67dc", | |||
| "metadata": {}, | |||
| "source": [ | |||
| "# Plotting OpenFE DG results against experiment using Cinnabar v0.4\n", | |||
| "# Plotting OpenFE DG results against experiment using Cinnabar v0.6.0\n", | |||
There was a problem hiding this comment.
This notebook is not included in the current docs do we still want this?
There was a problem hiding this comment.
It might be useful for people who run ABFEs not having to go throught he RBFE related content in the general plotting notebook, but maybe it's also not super important? If we keep it, we should definitely add it to the docs!
There was a problem hiding this comment.
Note: Personally I'm in support of merging the notebooks.
I think this notebook was necessary because some folks were asking how they could plot their absolute results and they didn't know how. We just need to make sure that's got a proper example somewhere.
| @@ -5,69 +5,29 @@ | |||
| "id": "ddae61fd-fa15-4be9-9160-ba3e195c67dc", | |||
There was a problem hiding this comment.
Maybe we could also add here or gathered with gather_abfe and refer to the ABFE CLI tutorial?
Reply via ReviewNB
| @@ -5,9 +5,9 @@ | |||
| "id": "ddae61fd-fa15-4be9-9160-ba3e195c67dc", | |||
There was a problem hiding this comment.
We probably need to make it clearer that this tutorial is geared solely towards relative free energies.
Reply via ReviewNB
| @@ -5,9 +5,9 @@ | |||
| "id": "ddae61fd-fa15-4be9-9160-ba3e195c67dc", | |||
There was a problem hiding this comment.
"gather the experiemtnal" -> typo
"to have units" -> Can you expand and mention OpenFF units here?
Reply via ReviewNB
| @@ -5,9 +5,9 @@ | |||
| "id": "ddae61fd-fa15-4be9-9160-ba3e195c67dc", | |||
There was a problem hiding this comment.
Line #8. value=row["DDG(i->j) (kcal/mol)"] * unit.kilocalories_per_mole, # the computed relative free energy change for the transformation
[nit] I like these inline comments, but it might be easier for folks to read them if you put the comment on the line before the kwargs rather than trying to have it all on the same line.
Reply via ReviewNB
| @@ -5,9 +5,9 @@ | |||
| "id": "ddae61fd-fa15-4be9-9160-ba3e195c67dc", | |||
There was a problem hiding this comment.
IAlibay
left a comment
There was a problem hiding this comment.
Couple of typos, otherwise the content changes look good.
I would advocate for merging post content fix and then opening a follow-up PR / issue to deal with the absolute cookbook (my suggestion would be to try to merge if possible, otherwise maybe breaking down the one tutorial into more cookbooks).
No description provided.