Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Include custom headers in model.cpp #132

Merged

Conversation

denisalevi
Copy link
Member

Fix for issue #131. I only included headers defined via prefs.codegen.cpp.headers.

Needed for `insert_code` statements which are included in `model.cpp`
and use custom headers.
@mstimberg
Copy link
Member

Thanks for the PR! I'm happy with the change in general, but for consistency with the other templates maybe rather use:

{% for name in user_headers | sort %}
#include {{name}}
{% endfor %}

? I don't think we need to support two types of syntaxes to specify headers here.
Also, maybe use user_headers = self.headers + prefs['codegen.cpp.headers'] as in the other places? I am not quite sure why we use self.headers elsewhere (it is an empty list anyway), but it might come in handy some day I guess.

@mstimberg
Copy link
Member

Oops, sorry... I just saw that the code I mentioned is used in brian2, not in brian2genn...

Copy link
Member

@mstimberg mstimberg left a comment

Choose a reason for hiding this comment

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

In the long-term we should make Brian2 and Brian2GeNN consistent with respect to the header syntax, but that's independent of this PR. Happy to merge things.

@mstimberg
Copy link
Member

Oh, but note that brian2genn does not work (yet) with brian2 master, due to the changes in the synapse generator.

@mstimberg mstimberg merged commit 0bb7f40 into brian-team:master Sep 7, 2021
@denisalevi denisalevi deleted the fix_headers_for_insert_code branch September 7, 2021 14:59
@denisalevi
Copy link
Member Author

Thanks for the quick merge and the hint! I will stick with the latest brian2 release for the benchmarks.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants