-
Notifications
You must be signed in to change notification settings - Fork 11
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 synapse clamping #492
Fix synapse clamping #492
Conversation
@@ -118,11 +118,11 @@ def add_stimuli( | |||
if data_stimuli is not None: | |||
externals["i"] = jnp.concatenate([externals["i"], data_stimuli[1]]) | |||
external_inds["i"] = jnp.concatenate( | |||
[external_inds["i"], data_stimuli[2].global_comp_index.to_numpy()] | |||
[external_inds["i"], data_stimuli[2].index.to_numpy()] |
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.
changed to .index
, since edges
does not have global_comp_index
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.
...and clamping of synapses needs the dataframe index.
fyi @michaeldeistler Also @huangziwei I noticed we have #363 still open? I think this should be easy to do #487 is merged. |
all_externals = list(self.externals.keys()) | ||
if "i" in all_externals: | ||
all_externals.remove("i") | ||
state_names = all_externals if state_name is None else [state_name] | ||
for state_name in state_names: |
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.
Took the liberty of making state_name optional. If none is supplied delete_clamps()
will just remove all clamps.
* add: wip version of tutorial on views * fix: added fixes for clamping and stimulating of synapses * fix: allow all states * fix: add current to states * fix: add tests for current and synapse clamping * fix: fix current clamp test * rm: rm accidently commited stash from different branch * enh: allow to remove multiple clamp * fix: make tests pass
Clamping of synaptic states was not tested and slipped through the cracks when #447 was merged. This lead to several issues #485.
This PR fixes this and adds test coverage.