-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Add prefix/postfix method to Equations #1051
Conversation
This gives an easy way to make a copy of an equation, while substituting some of the variable names.
Helpful for doctests, etc.
Could be internal/external instead of variables and constants? Also, maybe an exclude option in case you want to prefix almost everything except one variable? I'm wondering if we should have an argument names which can be either variables or constants rather than forcing the user to put things either in the constants or variables lists? So with the above suggested change, usage patterns might be:
Maybe we should make examples of using this feature part of the PR? Reworking existing examples ideally. Good to make docs not separate from code for PRs. Another approach that would be rather different would be to have a function that combines multiple Equations together rather than a method of Equations. The advantage here is that more contextual information would be available about what is defined elsewhere. So you'd call it as:
or something like that. The point is that like this, assuming |
Thanks, I also think that an
I'm not very happy with this, because I find it confusing if
Not very fond of this one, either :) I think having the "internal" variables being defined for a single set of equations is what we'd normally want. E.g. you have a general equation for a type of channel, and then postfix it with basic_eq = Equations('dv/dt = (g_L*(E_L - v) + currents)/Cm : volt')
synaptic_current = Equations('''I = g*(E - v) : amp
dg/dt = -g/tau : siemens''')
full_eq = (basic_eq +
Equations('currents = I_ge + I_gi : amp') + # any way to simplify this as well?
synaptic_current.postfix('_ge', external=True, exclude=['v']) +
synaptic_current.postfix('_gi', external=True, exclude=['v']))
print(full_eq) Output:
|
PS: I'm waiting with the documentation until we are sure about the syntax, don't want to write things twice :) |
[For the record, the test failure is just a build timeout which can be safely ignored.] |
@thesamovar : what do you think about the changes and my take on the |
I think we're still not quite there, but getting closer. Taking a look at this:
It seems more complicated than it needs to be. In particular, I find
That syntax is also not quite perfect, but perhaps that gets closer to the way a user might want to think about it? Like that the definition of
Or, make it more general again (and another idea for syntax):
That would allow prefix/postfix to be replace by
|
Ok, this is some good thinking. I like the I'm a bit undecided between the approach using an I have to say I'm not very fond of the general Oh, and a final possibility if we go for basic_eq = Equations('''
dv/dt = (g_L*(E_L - v) + currents)/Cm : volt
currents = I_ge + I_gi : amp
'''
)
synaptic_current = EquationsTemplate('''
{I} = {g}*({E} - v) : amp
dg/dt = -g/tau : siemens
''')
full_eq = (basic_eq +
synaptic_current.postfix('_ge') +
synaptic_current.postfix('_gi'))
print(full_eq) Oh, actually with this syntax we could even get rid of the basic_eq = Equations('''
dv/dt = (g_L*(E_L - v) + currents)/Cm : volt
currents = I_ge + I_gi : amp
'''
)
synaptic_current = '''
{I} = {g}*({E} - v) : amp
dg/dt = -g/tau : siemens
'''
full_eq = (basic_eq +
Equations(synaptic_current, postfix='_ge') +
Equations(synaptic_current, postfix='_gi'))
print(full_eq) Um, we keep adding options, we should probably try to converge on something... |
I think we're almost there for a solution. How about we have an
Good points - I definitely agree it shouldn't be our only general syntax. I wonder if it might occasionally be useful and less complicated that your dictionary based method though? On the one hand I think about just having multiple replace methods (maybe including a regexp based one), on the other hand perhaps there should just be one clear way of doing it, Zen of Python style? I don't think I think at this point, the best option to me looks like the
If you're OK with this, we still need to decide:
|
Maybe we should follow our usual approach and think in terms of use cases. What do you have in mind where the
Sounds good to me.
I think we could still have one automatic variant if no variables are specified at all (which wouldn't make any sense for a template). I think there are two possible choices: either all the variables defined in the equations (i.e. what we previously called "internal"), or everything (i.e. "internal" + "external"). I agree that the half-automatic
I'd say either a dictionary or keywords based (see my arguments above), keyword-based is maybe a bit nicer/more consistent with our usual style? What I mean is
I'd say we can leave this decision for a later time and focus on what we've discussed so far for now. I still see a potential usefulness in situations like this: if has_sodium_channels:
eqs += current.postfix('_Na')
# add I_Na to list of currents But then, this is something that is already pretty straightforward with standard Python, e.g. you could do: currents = 'I_pas'
if has_sodium_channels:
eqs += current.postfix('_Na')
currents += '+ I_Na'
# ...
eqs += Equations('currents = {} : amp'.format(currents)) or something like: currents = ['I_pas']
if has_sodium_channels:
eqs += current.postfix('_Na')
currents.append('I_Na')
eqs += Equations('currents = {} : amp'.format('+ '.join(currents)) |
Very good - you're right let's just leave that out entirely. I'm happy with either dictionary or keywords for Incidentally, It would also be nice to allow replacing a variable with a value, e.g. you might have So we do
I'm tempted to agree, but I wonder if we think of actual use cases that the default will never be useful. If you do only internal variables, then all constants and externally defined variables (like V) would be the same, so it would actually be the same. To make it worth doing, the constants have to be different. However, if you do all variables, then it's going to include V and functions as well, which is definitely not what would be expected and likely to lead to conclusion. So I think that actually fully explicit is the only coherent choice here.
Yep - I think it's sufficiently simple that we don't need to do anything ourselves here. OK, so I think the only issue remaining is whether or not to have default variables, and then we have a fully worked out proposal. Whatever we decide for that, I think we've iterated on a nice solution here. I'm pretty happy with it and I think it'll make life easier for a few people. |
We do actually have this already, which actually makes me realize that we only need the template approach for prefix/postfix and can forget about the general solution. From the docs: >>> general_equation = 'dg/dt = -g / tau : siemens'
>>> eqs_exc = Equations(general_equation, g='g_e', tau='tau_e')
>>> eqs_inh = Equations(general_equation, g='g_i', tau='tau_i')
>>> print(eqs_exc)
dg_e/dt = -g_e / tau_e : S
>>> print(eqs_inh)
dg_i/dt = -g_i / tau_i : S
>>> eqs = Equations('dv/dt = mu/tau + sigma/tau**.5*xi : volt',
... mu=-65*mV, sigma=3*mV, tau=10*ms)
>>> print(eqs)
dv/dt = (-65. * mvolt)/(10. * msecond) + (3. * mvolt)/(10. * msecond)**.5*xi : V There's also a Equations('dg/dt = -g / tau : siemens', g='g_e', tau=10*ms) you can also write: Equations('dg/dt = -g / tau : siemens').substitute(g='g_e', tau=10*ms) So the whole template thing would only be used for prefixing/postfixing. Which actually makes me wonder whether: basic_eq = Equations('''
dv/dt = (g_L*(E_L - v) + currents)/Cm : volt
currents = I_ge + I_gi : amp
'''
)
synaptic_current = Equations('''
I = g*(E - v) : amp
dg/dt = -g/tau : siemens
''').make_template('I', 'g', 'E')
full_eq = (basic_eq +
synaptic_current.postfix('_ge') +
synaptic_current.postfix('_gi')) is actually that much better than my initial implementation (discarding the internal/external stuff): basic_eq = Equations('''
dv/dt = (g_L*(E_L - v) + currents)/Cm : volt
currents = I_ge + I_gi : amp
'''
)
synaptic_current = Equations('''
I = g*(E - v) : amp
dg/dt = -g/tau : siemens
''')
full_eq = (basic_eq +
synaptic_current.postfix('_ge', variables=['I', 'g', 'E']) +
synaptic_current.postfix('_gi', variables=['I', 'g', 'E'])) This is more redundant due to its duplicated e_synapses = synaptic_current.postfix('_e', variables=['tau'])
i_synapses = synaptic_current.postfix('_i', variables=['tau'])
full_eq = (basic_eq +
e_synapses.postfix('_ampa', variables['I', 'g', 'E']) +
e_synapses.postfix('_nmda', variables['I', 'g', 'E']) +
i_synapses.postfix('_gaba', variables=['I', 'g', 'E'])) With the Oooh, that makes me think of another way to get rid of the redundancy without introducing a new class: what if you could give a list of prefixes/postfixes and get a list of e_synapse, i_synapse = synaptic_current.postfix(['_e', '_i'], variables=['I', 'g', 'E'])
full_eq = (basic_eq + e_synapse + i_synapse) |
Good to know that (again) past us is doing well. ;) I still don't like the repeat of the list of variables. Ideally, if we can find something without that would be good. One last argument in favour of EquationsTemplate: you could make it so that a template cannot be added to standard Equations and therefore reduce the chance of an error. Maybe that's not really worth worrying about though. OK assuming that we don't have EquationsTemplate but we also want to avoid repetition, it looks like we have two options. Either we go for an argument/method of Equations that marks certain variables to be replaced / as part of the template; OR we go with your list approach (which I agree is pretty neat). An argument against the list would be that it's less clear what it's doing without reading the documentation (not the worst thing in the world, but to avoid if possible). Another is that it promotes long lines of code, which is generally bad practice. I think my slight preference at this point is some way of marking certain variables to be replaced in Equations, maybe with a keyword |
I think I mostly agree with you, but I'm now again have some doubt about the real-word usefulness of what we discussed so far (sorry for going in circles here...). I had a look at the Rothman-Manis example, and tried to see what it would look like with a prefix/postfix syntax. The first two blocks of channel equations are: brian2/examples/frompapers/Rothman_Manis_2003.py Lines 48 to 68 in 64c1c02
I don't actually see anything here where we could use prefixes/postfixes! It would be helpful if instead of the values we had constants, but in this kind of model I think it makes sense to directly put the fixed parameters into the equations instead of having tons of constants. We can use the already existing substitution mechanism, but this does not make things much shorter or clearer, I think: eqs_gating = Equations('''dgating/dt = q10*(steady - gating)/tau : 1
steady = (1+exp((vu + v_s) / v_d))**exponent : 1
tau = ((a / (b1*exp((vu + v_s1) / v_d1) + b2*exp(-(vu+v_s1) / v_d2))) + c)*ms : second''')
# Classical Na channel
eqs_na = (Equations('ina = gnabar*m**3*h*(ENa-v) : amp') +
eqs_gating.substitute(gating='m', steady='m_inf', tau='tau_m',
v_s=38, v_d=-7, exponent=-1,
a=10, b1=5, v_s1=60, v_d1=18, b2=36, v_d2=25, c=0.04) +
eqs_gating.substitute(gating='h', steady='h_inf', tau='tau_h',
v_s=65, v_d=-6, exponent=-1,
a=100, b1=7, v_s1=60, v_d1=11, b2=10, v_d2=25, c=0.6))
# KHT channel (delayed-rectifier K+)
eqs_kht = (Equations('ikhtbar = gkhtbar*(nf*n**2 + (1-nf)*p)*(EK-v) : amp') +
eqs_gating.substitute(gating='n', steady='n_inf', tau='tau_n',
v_s=15, v_d=-5, exponent=-0.5,
a=100, b1=11, v_s1=60, v_d1=24, b2=21, v_d2=23, c=0.7) +
eqs_gating.substitute(gating='p', steady='p_inf', tau='tau_p',
v_s=23, v_d=-6, exponent=-1,
a=100, b1=4, v_s1=60, v_d1=32, b2=5, v_d2=22, c=5)) Now there is one place where something like prefix/postfix would make sense, the three names eqs_gating = Equations('''d{gating}/dt = q10*({gating}_inf - {gating})/tau_{gating} : 1
{gating}_inf = (1+exp((vu + v_s) / v_d))**exponent : 1
tau_{gating} = ((a / (b1*exp((vu + v_s1) / v_d1) + b2*exp(-(vu+v_s1) / v_d2))) + c)*ms : second''')
# Classical Na channel
eqs_na = (Equations('ina = gnabar*m**3*h*(ENa-v) : amp') +
eqs_gating.substitute(gating='m',
v_s=38, v_d=-7, exponent=-1,
a=10, b1=5, v_s1=60, v_d1=18, b2=36, v_d2=25, c=0.04) +
eqs_gating.substitute(gating='h',
v_s=65, v_d=-6, exponent=-1,
a=100, b1=7, v_s1=60, v_d1=11, b2=10, v_d2=25, c=0.6)) But then, is it really clearer than simply using Python's |
Hmm, maybe you're right - and it does allow more flexibility. Your example above might be simpler rewritten with eqs_gating ='''dgating/dt = q10*(gating_inf - gating)/tau_gating : 1
gating_inf = (1+exp((vu + v_s) / v_d))**exponent : 1
tau_gating = ((a / (b1*exp((vu + v_s1) / v_d1) + b2*exp(-(vu+v_s1) / v_d2))) + c)*ms : second'''
# Classical Na channel
eqs_na = (Equations('ina = gnabar*m**3*h*(ENa-v) : amp') +
Equations(eqs_gating.replace('gating', 'm')).substitute(
v_s=38, v_d=-7, exponent=-1,
a=10, b1=5, v_s1=60, v_d1=18, b2=36, v_d2=25, c=0.04) +
Equations(eqs_gating.replace('gating', 'h')).substitute(
v_s=65, v_d=-6, exponent=-1,
a=100, b1=7, v_s1=60, v_d1=11, b2=10, v_d2=25, c=0.6)) or we could imagine having some sort of wrapper around string functions to mildly simplify the syntax: eqs_gating = Equations('''dgating/dt = q10*(gating_inf - gating)/tau_gating : 1
gating_inf = (1+exp((vu + v_s) / v_d))**exponent : 1
tau_gating = ((a / (b1*exp((vu + v_s1) / v_d1) + b2*exp(-(vu+v_s1) / v_d2))) + c)*ms : second''')
# Classical Na channel
eqs_na = (Equations('ina = gnabar*m**3*h*(ENa-v) : amp') +
eqs_gating.string_replace('gating', 'm').substitute(
v_s=38, v_d=-7, exponent=-1,
a=10, b1=5, v_s1=60, v_d1=18, b2=36, v_d2=25, c=0.04) +
eqs_gating.string_replace('gating', 'h').substitute(
v_s=65, v_d=-6, exponent=-1,
a=100, b1=7, v_s1=60, v_d1=11, b2=10, v_d2=25, c=0.6)) |
Actually, looking again at your code with the Maybe we need more opinions here? |
I mean, from other people. |
So, that would mean that we actually abandon the whole prefix/postfix thing, right? I think of all the solutions so far, I'd prefer the variant with curly braces. I think having a tiny bit of syntactic sugar (i.e. you do not have to call I'll go ahead and implement this solution and convert Rothman&Manis, I think it is then easier to get feedback from others. This example would be another candidate for the approach. |
I don't think we will build upon the code in this PR any further, let's continue the discussion in #1244. |
Closes #1050
This should implement basically everything discussed in the PR. In addition to the prefix/postfix, the functions have two arguments:
variables
andconstants
, where you can give a list of variable or constant names (where constants is everything that is not defined within the equations) that you want to have prefixed. For convenience, you can giveTrue
orFalse
instead of a list, meaning "all" or "none" respectively. I think this is quite intuitive and matches what we do for therecord
argument of monitors. By default,variables=True, constants=False
.If you are happy with the general approach/syntax, then there are only a few minor things still to do:
constants
the best name, given that for an ion channel that does not definev
itself,v
would be part of the "constants"?exp
to becomeexp_postfixed
). User-defined functions would still be prefixed/postfixed if you doconstants=True
, but fixing this is a bit tricky and I think not worth the trouble for a not highly used feature. Maybe a note about this in the documentation would be enough?Here (from the docstring) how it works right now: