Skip to content
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

Finalize the refractoriness mechanism #66

Merged
merged 11 commits into from Jul 4, 2013
Merged

Conversation

mstimberg
Copy link
Member

This should implement the new refractoriness mechanism that came out of our discussion. The major differences to the previous (Brian2) implementation: The flag is called unless-refractory instead of active, the variable storing the state of neurons is called not_refractory instead of is_active. Specifying the length of the refractory period works with a new refractory keyword in NeuronGroup, it accepts a duration or a string that evaluates to a duration or a string that evaluates to a boolean (the condition to stay in the refractory state after a spike). Closes #41
Also includes some tests and a bit of "user documentation".

@ghost ghost assigned thesamovar Jul 1, 2013
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ca65d1e on refractoriness_final into ee7ece5 on master.

@mstimberg
Copy link
Member Author

Coverage remained the same when pulling ca65d1e on refractoriness_final into ee7ece5 on master.

I'm not quite sure why the coverage is "unknown" on this branch, but let's ignore this for now...

@thesamovar
Copy link
Member

A couple of questions before merging:

not_refractory feels somehow not as nice as is_active, but I don't see a better alternative if we're removing the notion of 'active' (which I agree with as its not intuitive enough).

it accepts a duration or a string that evaluates to a duration or a string that evaluates to a boolean

How does the string evaluating to a duration or boolean work? Can the string use the state variables, time, etc.? How do you parse which it is?

@mstimberg
Copy link
Member Author

not_refractory feels somehow not as nice as is_active, but I don't see a better alternative if we're removing the notion of 'active' (which I agree with as its not intuitive enough)

Yeah, I'm not too fond of not_refractory either -- but since there is no commonly used word for the opposite of refractory, I think this is the best choice. It is quite clear and doesn't introduce any new term. I'll ask Romain for feedback on this, though.

it accepts a duration or a string that evaluates to a duration or a string that evaluates to a boolean

How does the string evaluating to a duration or boolean work? Can the string use the state variables, time, etc.? How do you parse which it is?

Yes, it can use everything. I'm not actually parsing it but I'm using the unit-checking mechanism for expressions (which evaluates the expression, after substituting all external variables/state variables with their unit). If it results in a quantity with unit second it is considered a time expression, if it is a dimensionless quantity (or array/scalar), it is considered a boolean. If it is a quantity with a non-time unit, an error is raised. Treating every dimensionless value as boolean is certainly not foolproof -- I'm wondering whether maybe introducing a special boolean "unit" would make sense here, we could then detect it properly.

@thesamovar
Copy link
Member

No, let's not abuse the units system to put booleans in there (although with regard to the other thread, writing bool in the unit slot seems fine). If we know which variables are booleans, with the ast parser it's pretty easy to determine what is a boolean expression. We simply search through the tree and check at each level that EITHER the operation is logical (and or) and each of the arguments are logical recursively OR the operation is an inequality OR it is a variable with boolean type.

@mstimberg
Copy link
Member Author

If we know which variables are booleans, with the ast parser it's pretty easy to determine what is a boolean expression. We simply search through the tree and check at each level that EITHER the operation is logical (and or) and each of the arguments are logical recursively OR the operation is an inequality OR it is a variable with boolean type.

In general this sounds good. However the advantage of evaluating (as it is done in the unit-checking) is that in can deal with arbitrary functions, as long as those functions manage to deal with pure units. E.g. this would work right now (ignoring the problem of specifying the function for C++):

def in_refractory(t, lastspike):
    return (t - lastspike) > 5*ms 

G = NeuronGroup(...refractory='in_refractory(t, lastspike)'...)

In the above example there's of course no need for a function in the first place, but I like the generality of the mechanism. There are some other issues with units and functions that prevent that from working ATM, though.

@thesamovar
Copy link
Member

That's true, but on the other hand decorating the function with @returns_bool or something like that isn't so bad. OK, let's stick with your mechanism for the moment, but it would be nice to have a more consistent one for the future.

@mstimberg
Copy link
Member Author

That's true, but on the other hand decorating the function with @returns_bool or something like that isn't so bad. OK, let's stick with your mechanism for the moment, but it would be nice to have a more consistent one for the future.

I'm not really attached to the eval technique, executing user-provided functions might have unintended side-effects. I think we discussed using the @check_units decorator to specify units (in a more flexible way, allowing to specify a function of input units to calculate the output unit), probably that could be extended to specify boolean inputs/outputs. With this mechanism in place, both options ("evaling" and parsing) would work.

@thesamovar
Copy link
Member

I like that idea, it feels more consistent and less ad hoc and error prone. It fits with our general strategy of trying not to do quite as much guessing as we did in Brian 1 to avoid difficult to debug errors. So the proposal would be: whenever a function is used, the user has to declare with @check_units the return unit or whether it's a bool? Not just for refractoriness but each time a function is used in a string.

@mstimberg
Copy link
Member Author

So the proposal would be: whenever a function is used, the user has to declare with @check_units the return unit or whether it's a bool? Not just for refractoriness but each time a function is used in a string.

I think that would be a good idea -- slightly annoying for some users, maybe, but it's the only way to make unit-checking work reliably and simple. Most user functions are quite simple in their inputs/outputs anyway (and even more users never use such user-defined functions in the first place). We could use None as a wildcard for "any unit" and use a functional form for the output argument (that gets the units of the arguments as inputs). So a function that returns time-dependent voltage would use @check_units(t=second, result=volt) and a simple square function would use @check_units(x=None, result=lambda x_unit: x_unit**2).

@thesamovar
Copy link
Member

I like it!

@mstimberg
Copy link
Member Author

I opened an issue for the user-provided functions as #67 -- we should deal with this independently of the refractoriness.

@thesamovar
Copy link
Member

Shall I merge this now or did you want to finish anything up?

@mstimberg
Copy link
Member Author

Shall I merge this now or did you want to finish anything up?

Not working on anything in this branch right now, good to merge from my side.

thesamovar added a commit that referenced this pull request Jul 4, 2013
Finalize the refractoriness mechanism
@thesamovar thesamovar merged commit 32897cf into master Jul 4, 2013
@thesamovar thesamovar deleted the refractoriness_final branch July 4, 2013 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Finalize refractoriness system
3 participants