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.

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

Check for multiple synapses #39

Merged
merged 3 commits into from
Mar 20, 2017
Merged

Check for multiple synapses #39

merged 3 commits into from
Mar 20, 2017

Conversation

mstimberg
Copy link
Member

As stated in #10, Brian2GeNN does not correctly handle multiple connections between a pair of neurons (which is a limitation of GeNN itself, I think?). There's no very clean solution to handle this case, since we don't now whether there are any multisynapses until we have generated them all. The approach in this PR is to check during the conversion of "Brian synapses" to "GeNN synapses". To give a useful error message on the Python level, the code will not only print out an error message but will also exit with a custom error level that is then processed on the Python side and raises a NotImplementedError.

Not sure whether you are happy with my check for multiple synapses, though. Performance-wise it should not have a noticeable effect, I think.

@mstimberg mstimberg force-pushed the check_for_multiple_synapses branch from 72012bc to bc523d6 Compare March 6, 2017 16:14
…or code to raise a NotImplementedError on the Python side

Also change tabs to spaces in convert_synapses.h
@mstimberg mstimberg force-pushed the check_for_multiple_synapses branch from bc523d6 to 34e2b58 Compare March 7, 2017 15:15
@tnowotny
Copy link
Contributor

hum ... what happened to the tests?

@mstimberg
Copy link
Member Author

hum ... what happened to the tests?

The test failures where due to other problems not related to the changes in the branch. They were already fixed in master, so after merging master into this branch, all tests now pass.

@tnowotny tnowotny merged commit e70329f into master Mar 20, 2017
@mstimberg mstimberg deleted the check_for_multiple_synapses branch March 20, 2017 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants