Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix rev in syn #520

Merged
merged 2 commits into from
May 3, 2022
Merged

Fix rev in syn #520

merged 2 commits into from
May 3, 2022

Conversation

neworderofjamie
Copy link
Contributor

Fixes #519

tnowotny and others added 2 commits April 29, 2022 20:47
@tnowotny
There was a small bug where the name of the FusedPreOutputSuffix was …
17a8844 
…not initialised when a new synapsegroup was created.

This is now fixed but it has revealed another bug where an initgroup is created including synapses with and without revInSyn.
the generated push for the non-existing revInSyn then makes compilation fail. I will raise an issue.
@neworderofjamie
99% sure the merging decision was correct as this field isn't require…
729b1d5 
…d during synapse initialisation (because they're potentially merged, they're initialised in the neuron initialisation code). Instead the problem was that this field was being added incorrectly to the merged synapse initialisation structures - now it only gets added to synapse *update* structures
@neworderofjamie neworderofjamie added this to the GeNN 4.7.2 milestone May 3, 2022
@neworderofjamie neworderofjamie requested a review from tnowotny May 3, 2022 08:17
tnowotny
tnowotny approved these changes May 3, 2022
View reviewed changes
Copy link
Member

@tnowotny tnowotny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work well and makes sense I think. Thanks for sorting that out.

@neworderofjamie neworderofjamie merged commit 93a50ca into master May 3, 2022
@neworderofjamie neworderofjamie deleted the fix_revInSyn branch May 3, 2022 14:54
@neworderofjamie neworderofjamie modified the milestones: GeNN 4.7.2, GeNN 4.8.0 May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnowotny tnowotny tnowotny approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problems with revInSyn and merged synapse init groups
2 participants
@neworderofjamie @tnowotny