-
Notifications
You must be signed in to change notification settings - Fork 216
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
New run system (changed namespace handling) #34
Conversation
pre_run() and post_run()
namespace defined during its creation, the namespace surrounding or provided to the run function will be used.
_find_names (even though I still have no idea why it should change in the first place...)
namespace is complete for the object).
corresponding sympy function, used in parse_to_sympy to not treat exp, sin, etc. as undefined, arbitrary functions. This might allow for simplifications in the code. brian.utils.parsing had to move to brian2.codegen to make this work because it depends on brian2.codegen.functions.numpyfunctions now.
sympy to replace expressions like sqrt(x**2) by abs(x), for example.
NeuronGroup-specific in it, move it to "GroupCodeRunner") and document and test a bit more.
I have carefully reviewed the... text above, and find it to be very worthy of merging. There is a build failure for Python 3.3, do you want to fix that or shall I just go ahead? |
Yes, I saw that before -- the problem seems to be coming and going :-/ It's some subtlety of the magic system (maybe some garbage collection detail changed in 3.3?), I don't think I'll fix that soon. Given that 3.2 seems to be working fine, I'd say go ahead with the merge. |
New run system (changed namespace handling)
This closes #31
Namespaces
As discussed, there are now three different options for specifying namespaces:
NeuronGroup
-- this is fully explicit nothing else will be used for namespace lookup (except for functions and units)You cannot override functions or units via either of these methods, they always take precedence.
For all of these, the namespace does not have to be complete at the time of NeuronGroup creation (not relevant for point 2 above).
For example you can do:
Or
You can change elements in the namespace between runs, the implicit namespace always takes the context at the point of the run call into account.
Unit checking
Units are checked at creation time, when the namespace is already complete. This is done in a very simple way by just trying it and silently ignoring KeyErrors (resulting from a namespace lookup failure). In the case of an incomplete namespace and unit errors in the model it makes the result unfortunately somewhat unpredictable: Depending on the order of unit checks the check might be raised (if it occurs before any KeyError) or not (if it occurs after a KeyError).
At the point of the run call, all units are checked and namespace resolution failures lead to an error, so unit errors should never slip through, even for changes in the namespace between runs.
BTW: Along the way I introduced unit checking for thresholds and resets! It checks directly the abstract code and even deals with new temporary variables. I first thought that the same check would be useful for the state updater code (it would spot errors in state updater descriptions) but then I realized that the linear state updater already gets rid of some units because it substitutes constants from the namespace, so I disabled the check for the state updater. The equations itself are checked anyway. We could add a check to the
ExplicitStateUpdater
class though that checks the general formulation for errors.Some more remarks: Because the abstract code for state updates, resets and thresholds is regenerated for each run it is now possible to change the reset or threshold definition between runs.
All the "change between run" features are not much tested in the test suite but it should be quite save as there is no fundamental difference between the first run and subsequent runs in the code.
Finally, I did get rid of the
CodeRunner
vs.NeuronGroupCodeRunner
distinction and moved the new class toGroupCodeRunner
as there is nothingNeuronGroup
specific about it anymore. Apart fromSynapses
it might be useful forStateMonitor
, for example?Sooo... anything missing? Otherwise I think it's good to merge (after a scrutinizing review, of course ;) ).