-
Notifications
You must be signed in to change notification settings - Fork 54
Add numeric solver to synapses #1208
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
base: master
Are you sure you want to change the base?
Conversation
y += [syn_stats["y"]] | ||
z += [syn_stats["z"]] | ||
|
||
# TODO: Adjust tolerance |
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.
Could you take care of this TODO?
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 tolerance is a bit high. The tests fail if I decrease the tolerance. I tried a few things, such as adjusting rtol
and atol
, but couldn't lower the difference. Do you have any ideas?
pynestml/codegeneration/resources_nest/point_neuron/common/SynapseHeader.h.jinja2
Outdated
Show resolved
Hide resolved
@@ -294,7 +294,7 @@ def transform_neuron_synapse_pair_(self, neuron: ASTModel, synapse: ASTModel): | |||
strictly_synaptic_vars = ["t"] # "seed" this with the predefined variable t | |||
|
|||
if self.option_exists("strictly_synaptic_vars") and removesuffix(synapse.get_name(), FrontendConfiguration.suffix) in self.get_option("strictly_synaptic_vars").keys() and self.get_option("strictly_synaptic_vars")[removesuffix(synapse.get_name(), FrontendConfiguration.suffix)]: | |||
strictly_synaptic_vars.append(self.get_option("strictly_synaptic_vars")[removesuffix(synapse.get_name(), FrontendConfiguration.suffix)]) | |||
strictly_synaptic_vars.extend(self.get_option("strictly_synaptic_vars")[removesuffix(synapse.get_name(), FrontendConfiguration.suffix)]) |
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.
This was a bug, right? How could this ever have worked before?
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 synapse model used in the test in this PR has variables that strictly belong to the synapse. Hence, I added them to strictly_synaptic_vars
in the codegen_opts introduced in #1229.
This then caused the append
in the line to create a list of lists, which failed at line:
nestml/pynestml/utils/ast_utils.py
Line 1779 in 81ed384
assert type(var) is str |
That's why I changed it to
extend
. Perhaps we should change all the append
calls here to extend
instead to avoid errors.
Could you also update |
No description provided.