Open ajkluber opened 8 years ago
In short, I think it's an okay modification ,but I have some reservations on the implementation that I think we should consider.
When we first set out to do the refactor, we were aiming to make the package more similar to the pyemma
package. I think the philosophy of that package has been that the built in functions can do all the heavy lifting, but specific little tinkering things like adding a potential here, or there, is left to the user to add in some way. My understanding of this "some way" meant the user would have their python script, and inside the script they would load the model with the inputs.py, and then do any modifications to the model followed by using the writer objects to output the gromacs files.
So what I imagine would your current python script is something like this:
import model_builder
model = model_builder.inputs.load_model("bpti.ini")
model.add_umbrella_potential(minima, K, *args)
And essentially the augment_model object would just carry out the model.add_umbrella_potential
function. A more meaningful example of what I mean would be something like:
import model_builder
model = model_builder.inputs.load_model("bpti.ini")
elastic_network_model_contacts = [ ... ]
for enm in elastic_network_model_contacts:
model.add_harmonic_potential(*args)
Then this would make more sense to want to handle it automatically, as how you construct the enm might change a lot depending on your specific model (Ca vs CaCb, cutoff distances, etc.). We don't need to add a helper function in the model
objects just to add the harmonic potential, but for example it could be, model.Hamiltonian.add_potential("harmonic", *args)
.
Some thoughts on this:
inputs.py
scripts is basically our automated way to make a model, so it is appropriate to include the option to supply the augment_model
function. Especially in a case of an ENM, you might want to automatically determine which ENM based on some criteria based on the mapping, so you include the option to do so an call the model to add the necessary potentials.I should provide a little more context that led me to this idea. I was using my inherent_structure
package that loads in a model
in the usual way in the source which doesn't allow me to add these extra interactions before the calculation runs (as I normally would in the manner you suggested if I was running simulation). In a similar way, this would be a problem for any external program that wants to create a model
from just an .ini
file.
In response to your thoughts:
model = augment_model(model)
could break later functionality in an unforeseeable way makes this especially tricky. There are some checks for supported functions in the output
writers, but this could yield some bugs that are hard to track down..ini
file to be a self-contained way to create a model
. I liked the idea of listing a script as an argument in the .ini
so that the model
could be changed in different contexts if necessary. These modifications could either be "extra" pieces to the Hamiltonian, like an umbrella potential, or just some custom interaction as I mention above. I will wait to implement this for the time being.
I want to add a feature that allows the user to modify the model object (e.g. by adding interactions) using an external python script. I am proposing a solution but I want to get feedback (mainly from @TensorDuck ) about how to implement it without disrupting recent changes to
inputs.py
.I have found this to be necessary when creating the model of SNARE that @gsc4 has been using, which required a harmonic interaction between the endpoints to model the effects of the pulling potential. However, I think this solution will solve all these kinds of 'tinkering' problems we may have.
Specifically, I want to add an argument to the
.ini
file that defines the path to an external python script that the user has created (e.x.path_to_py = /home/ajk8/scratch/modify.py
). The external script has a function that takes in amodel
object and returns amodel
object.I basically just want to add the following code to
inputs.py
,