search

alchemistry / alchemtest

the simple alchemistry test set
https://alchemtest.readthedocs.io
BSD 3-Clause "New" or "Revised" License
8 stars 12 forks source link

issue-4-gromacs-expanded-ensemble-data #16

Closed trje3733 closed 6 years ago

trje3733 commented 6 years ago

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.

orbeckst commented 6 years ago

Sorry, didn't see the PR. Please feel free to assign me as a reviewer (then I get notified) or ping me with @orbeckst.

trje3733 commented 6 years ago

Thanks for the review. Made the changes and added a REX dataset.

orbeckst commented 6 years ago

@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.

trje3733 commented 6 years ago

@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.

orbeckst commented 6 years ago

@trje3733 looking good. If you want to add more context (see https://github.com/alchemistry/alchemtest/pull/16#discussion_r153871782 ) with citations and descriptions then please go ahead. I'll merge tomorrow (or whenever you tell me that you're ready).

trje3733 commented 6 years ago

@orbeckst Sounds good, I'll add that information later today and let you know.

trje3733 commented 6 years ago

@orbeckst I added citations. Let me know if you find any issues. Thanks, Travis.

orbeckst commented 6 years ago

Excellent, merging now.

orbeckst commented 6 years ago

Congratulations @trje3733 , first PR merged! Good work.