-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fix compilation error when synapse model uses random_normal
function
#805
base: master
Are you sure you want to change the base?
Conversation
Note: #806 needs to be fixed before this is merged. |
return tid; | ||
} | ||
{%- endif %} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we just use kernel().vp_manager.get_thread_id()
directly wherever this is needed? I'm not sure I like this additional indirection... Optimally, I think get_thread_id()
should just be available as an API function in the nest
namespace defined in nestkernel/nest.h
in NEST Simulator proper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated code for the random_normal()
function from NESTML is the same for both neurons and synapses. See here. The neuron already has an implementation of the get_thread()
function in its base class whereas the synapse doesn't. Hence this PR adds a private implementation for the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should synapses instead use the thread t
that is passed as a parameter to the send()
method, rather than calling kernel().vp_manager.get_thread_id()
? (cc @jougs)
This needs to be fixed from the NEST end. The corresponding issue in NEST: nest/nest-simulator#2902 |
Could we for now add a context condition check, specific to the NEST target, that checks if these rng functions are used in the state, parameters and internals blocks of synapse models, and errors out in that case? Just to prevent people from getting unexpected results. |
Fixes #804
Unlike the neuron class, the synapse class in the generated code does not have an implementation of the
get_thread()
function that is used innest::normal_distribution
. This PR adds a private implementation of the function and returns the thread id from the NEST kernel.