issue-4-gromacs-expanded-ensemble-data #16
Conversation
|
Sorry, didn't see the PR. Please feel free to assign me as a reviewer (then I get notified) or ping me with @orbeckst. |
src/alchemtest/gmx/access.py
Outdated
| DESCR=fdescr) | ||
|
|
||
| def load_expanded_ensemble_case_2(): | ||
| """Load the Gromacs Host CB7 Guest C3 expanded ensemble dataset, case 2 (two simulations visit all states independetly). |
src/alchemtest/gmx/__init__.py
Outdated
| @@ -3,3 +3,5 @@ | |||
| """ | |||
|
|
|||
| from .access import load_benzene | |||
| from .access import load_expanded_ensemble_case_1 | |||
There was a problem hiding this comment.
let's do imports on a single statement (not sure why we had it differently elsewhere... maybe @dotsdl 's preference??)
from .access import (load_benzene, load_expanded_ensemble_case_1,
load_expanded_ensemble_case_2, )|
|
||
| Host CB7 and Guest C3 in water, Guest C3 alchemically turned into Guest C3 in vacuum separated from water and Host CB7 | ||
|
|
||
| This dataset was generated using the expanded ensemble aglorithm in the `Gromacs <http://www.gromacs.org/>`_ molecular dynamics engine. |
|
|
||
| Host CB7 and Guest C3 in water, Guest C3 alchemically turned into Guest C3 in vacuum separated from water and Host CB7 | ||
|
|
||
| This dataset was generated using the expanded ensemble aglorithm in the `Gromacs <http://www.gromacs.org/>`_ molecular dynamics engine. |
|
Thanks for the review. Made the changes and added a REX dataset. |
| @@ -0,0 +1,22 @@ | |||
| Gromacs: Host CB7 and Guest C3 in water | |||
| ========================= | |||
There was a problem hiding this comment.
Please extend the ==== line to be at least as long as the text. This must be legal restructured text section header to be correctly type set as documentation.
| :Date: November 2017 | ||
| :License: `CC0 <https://creativecommons.org/publicdomain/zero/1.0/>`_ Public Domain Dedication | ||
|
|
||
| Host CB7 and Guest C3 in water, Guest C3 alchemically turned into Guest C3 in vacuum separated from water and Host CB7 |
There was a problem hiding this comment.
Do you have a paper that should be cited with the data? You can include it in the same way as in the Amber test data https://raw.githubusercontent.com/alchemistry/alchemtest/master/src/alchemtest/amber/simplesolvated/descr.rst
There was a problem hiding this comment.
The data generated has not been used in a publication but the system was part of the SAMPL4 challenge and similar simulations on this system were performed by a student of our group for a publication. Would you suggest citing one of these?
There was a problem hiding this comment.
In general I like context. You could state where the systems came from (citing the appropriate SAMPL4 host/guest overview paper... assuming that there is one) and saying that the data are similar to the ones in cite paper from your group.
There was a problem hiding this comment.
However, I am not aware of a publication that used replica exchange on this system.
You can always say that these are unpublished data.
My primary concern is simply that right now is probably the only time ever that someone (i.e., you) is looking closely at the data here and the associated meta-data so now is the time to provide any useful information. In principle it is not super-important to know everything because you are the expert who is going to write the tests anyway and you know your system. However, if anyone else wants to use the test data to test their own implementations then this additional information can come in handy.
Use your own judgement. In principle I am happy with the PR.
|
@trje3733 I noticed that you created the PR from your master branch. That will work but is likely going to create problems for you in the future (as soon as I squash and merge this PR) because at this point in time, your master and the alchemistry master will have diverged. For the future, have a look at How to do PRs on the wiki: basically, do PRs from a branch. Let me know once you have attended to the minor revisions – just ping me and I will get the PR merged. You'll then be able to hack away at alchemistry/alchemlyb#14. |
|
@orbeckst thanks for the suggestions for doing PRs. Still learning the ropes of efficiently using GitHub. I've modified the titles but have not added a citation yet (this exact data was not used in a publication but expanded ensemble simulations were previously performed on this system in a publication). However, I am not aware of a publication that used replica exchange on this system. Let me know if this publication should be cited and I can make those changes. |
|
@trje3733 looking good. If you want to add more context (see #16 (comment) ) with citations and descriptions then please go ahead. I'll merge tomorrow (or whenever you tell me that you're ready). |
|
@orbeckst Sounds good, I'll add that information later today and let you know. |
|
@orbeckst I added citations. Let me know if you find any issues. Thanks, Travis. |
|
Excellent, merging now. |
|
Congratulations @trje3733 , first PR merged! Good work. |
Provides two sets of expanded ensemble simulation data. Case 1 is a single simulation dataset that visits all states. Case 2 is two simulation datasets, each of which visits all states. Additional sets of data to come (REX and multiple expanded ensemble simulations that sample all states only when combined).
PR is related to issue #4 of alchemtest and will provide sample data for issue alchemistry/alchemlyb#14 of alchemlib.