Added before_run codeblock in numpy and C++ templates#1603
Added before_run codeblock in numpy and C++ templates#1603mahipalimkar wants to merge 3 commits intobrian-team:masterfrom
Conversation
|
Apologies, I did not yet have time to have a look at the PR, but I will do so tomorrow ! |
mstimberg
left a comment
mstimberg
left a comment
There was a problem hiding this comment.
This is a good start! I only looked at the Python code so far, and there are a number of issues that prevent the code from running – I cannot yet judge whether it will actually work after fixing these issues, but let's take it one step at a time 😊
| @@ -1,25 +1,48 @@ | |||
| {# USES_VARIABLES { _spikespace, neuron_index, _timebins, _period_bins, _lastindex, t_in_timesteps } #} | |||
| {% extends 'common_group.py_' %} | |||
| {% extends 'common_group.py.jinja2' %} | |||
There was a problem hiding this comment.
It might have been better to call templates like this, but it is currently called common_group.py_.
|
|
||
| {% block before_code %} | ||
| # Copy of the SpikeGeneratorGroup.before_run code | ||
| dt = {{dt.item()}} # Always copy dt |
There was a problem hiding this comment.
These variables are not available to the template, since it does not know that they are needed. You can use USES_VARIABLES to specify them. See https://brian2.readthedocs.io/en/stable/developer/codegen.html#template-keywords and other templates.
| # Always recalculate timesteps | ||
| from brian2 import defaultclock # Use Brian 2's clock instead of 't' | ||
| current_t = defaultclock.t | ||
| timesteps = ({{_spike_time}} / dt).astype(np.int32) |
There was a problem hiding this comment.
The names used here have to be the names as added with self.variables.add_, not the name of the SpikeGeneratorGroup attribute – in this case, the name should be spike_time not _spike_time.
| {% block before_code %} | ||
| # Copy of the SpikeGeneratorGroup.before_run code | ||
| dt = {{dt.item()}} # Always copy dt | ||
| period = {{period_}} # Always copy period |
There was a problem hiding this comment.
The _ suffix syntax is only used in Python user code to get the values of a variable without its units. In a template, variables are always the underlying numpy arrays without unit information, you can therefore simply use period here.
| period = {{period_}} # Always copy period | |
| period = {{period}} # Always copy period |
| {% block maincode %} | ||
| _the_period = {{_period_bins}} | ||
| _timebin = {{t_in_timesteps}} | ||
| _timebin = int(defaultclock.t / {{dt.item()}}) # Use Brian 2's clock instead of 't_in_timesteps' |
There was a problem hiding this comment.
See comment about .item() above.
|
|
||
| # Always recalculate timesteps | ||
| from brian2 import defaultclock # Use Brian 2's clock instead of 't' | ||
| current_t = defaultclock.t |
There was a problem hiding this comment.
It should use {{t}} instead of the defaultclock.t (even though most of the time the two will be the same).
| current_step = int(current_t / dt) | ||
|
|
||
| # Always update _lastindex | ||
| in_the_past = np.nonzero(timesteps < current_step)[0] |
There was a problem hiding this comment.
In the Python templates, numpy is available as _numpy, not np.
| in_the_past = np.nonzero(timesteps < current_step)[0] | |
| in_the_past = _numpy.nonzero(timesteps < current_step)[0] |
|
|
||
| # Always recalculate _timebins | ||
| shift = 1e-3 * dt | ||
| timebins = np.asarray(({{_spike_time}} + shift) / dt, dtype=np.int32) |
There was a problem hiding this comment.
| timebins = np.asarray(({{_spike_time}} + shift) / dt, dtype=np.int32) | |
| timebins = _numpy.asarray(({{_spike_time}} + shift) / dt, dtype=np.int32) |
| # Always update _lastindex | ||
| in_the_past = np.nonzero(timesteps < current_step)[0] | ||
| if len(in_the_past): | ||
| {{_lastindex}}[0] = in_the_past[-1] + 1 |
There was a problem hiding this comment.
For scalar variables, Brian already applies the [0] automatically.
| {{_lastindex}}[0] = in_the_past[-1] + 1 | |
| {{_lastindex}} = in_the_past[-1] + 1 |
|
Thank you so much @mstimberg ! I can see I made a lot of errors 😓 I will make these changes at the earliest |
|
Working with this mix of Python code, Jinja templates, Brian's idiosyncratic variable system, etc. is really far from straightforward – I wouldn't expect anyone to get this right on the first try! For testing whether things work, try running any example that uses |
|
Sorry for the gap in communication @mstimberg ! I have started incorporating all the changes that you suggested. Will do this at the earliest. |
|
Hi @mahipalimkar, do you think you will have time to pick up the work on this PR any time in the upcoming weeks? |
2 participants
Moved the Python code in
SpikeGeneratorGroup.before_runinto thebefore_codeblocks of the numpy and C++ templates.