choderalab / YankTools

Test systems for Yank
GNU General Public License v2.0
0 stars 2 forks source link

Consider generalizing this to `openmm-utilities` #7

Closed kyleabeauchamp closed 10 years ago

kyleabeauchamp commented 10 years ago

Depending on how our various refactors go, we might find that there are additional key functionalities that ought to be abstracted away into a separate repo. If that ends up happening, we could consider putting them here and changing the name to something like openmm-utiltiies. Then testsystems would just be one module in openmm-utilities.

Let's just keep this idea on the backburner in case it becomes useful later on. We won't know until we hack away at everything a bit more...

kyleabeauchamp commented 10 years ago

One reason I bring this up is that we have massive code duplication throughout Yank, Repex, pymbar, etc. Things like ThermodynamicState have been defined in each of those codebases.

IMHO, it's better to create a shared "utilities" library than to maintain five different versions. My real worry is that one package might change the interface without telling the others, leading to a dependency hell...

jchodera commented 10 years ago

We definitely want to get rid of code duplication when possible. The idea of ThermodynamicState here is just having a useful container object that shares an interface with the repex version of ThermodynamicState. (This would be easy in Java!) They don't have to be the same object, but they have to provide some of the same attributes.

Generalizing this to a utility repository (rather than a testing repository) may be a good idea.

What would we include?

Is there a big advantage to a "everything but the kitchen sink" utility module, rather than more smaller, focused modules?

Also, how would we import it? I don't think we can use dashes in import paths. Maybe choderalab.openmm.utilities?

kyleabeauchamp commented 10 years ago

Yeah, we need to replace dashes with underscores.

I think the big advantage of "everything but the kitchen sink" is to avoid making people install large numbers of prerequisites. I think people will get pissed if we have 3 "mini-repos" that they have to install. I think it also will help us out in terms of version numbering etc.

jchodera commented 10 years ago

Sounds reasonable. Let's do that for now.

Maybe we can:

kyleabeauchamp commented 10 years ago

I personally think we should avoid having a 4-level hierarchy. For example, in the simtk tools, there are no docstrings or tab-completing at the highest level:

In [3]: simtk?
Type:       module
String Form:<module 'simtk' from '/home/kyleb/opt/lib/python2.7/site-packages/simtk/__init__.pyc'>
File:       /home/kyleb/opt/lib/python2.7/site-packages/simtk/__init__.py
Docstring:  <no docstring>

In [6]: simtk.
kyleabeauchamp commented 10 years ago

Here's another possible name: yanktools

The advantages are:

  1. Promotes the Yank brand
  2. People will associate Yank and YankTools, so it's will seem natural to require YankTools as a prerequisite for Yank
jchodera commented 10 years ago

Sure, yanktools works for now. Go for it.

kyleabeauchamp commented 10 years ago

Two more questions to discuss:

  1. Should yanktools contain repex?
  2. What exactly should yanktools contain? (Thermodynamics, testsystems, integrators, what else...)
jchodera commented 10 years ago

What if repex contained what we were discussing putting in yanktools?

We are talking about:

It seems natural to just put all this in repex for now.

kyleabeauchamp commented 10 years ago

Yeah that might be the most convenient option. On Nov 10, 2013 7:21 PM, "John Chodera" notifications@github.com wrote:

What if repex contained what we were discussing putting in yanktools?

We are talking about:

  • ThermodynamicState (which repex needs anyway)
  • test systems with analytical results to validate sampling is correct
  • test systems without analytical results to make sure repex runs correctly
  • various integrators
  • MCMC methodology

It seems natural to just put all this in repex for now.

— Reply to this email directly or view it on GitHubhttps://github.com/choderalab/YankTools/issues/7#issuecomment-28165388 .

kyleabeauchamp commented 10 years ago

Consider this repo deprecated...

Everything has been moved to https://github.com/choderalab/Repex/