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

Cython: Include support code in before_run block #1428

Merged
merged 1 commit into from Jul 27, 2022

Conversation

mstimberg
Copy link
Member

Support code has to be included for before_run blocks, otherwise variables needed for functions that refer to namespaces (e.g. TimedArray) will not be correctly defined. This is a simple fix, in the longer run we should think about custom functions, namespaces etc. in before_run block. Currently, these blocks do not actually need any of all this, since they don't run user-defined code (but potentially that might change in the future?).

Fixes #1427, see https://brian.discourse.group/t/time-dependend-input-in-equation-vs-parameter/753/5 for example code that failed previously.

Otherwise, variables needed for functions that refer to namespaces
(e.g. TimedArray) will not be correctly defined.
Fixes #1427
@mstimberg
Copy link
Member Author

@schmitts the fix turned out to be almost trivial. Could you try out the fix_functions_namespace_cython branch with your code to confirm that it works correctly?

Copy link
Member

@thesamovar thesamovar left a comment

Choose a reason for hiding this comment

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

This looks fine to me! Not sure if you needed a review, but doing one anyway. 😉

@mstimberg
Copy link
Member Author

This looks fine to me! Not sure if you needed a review, but doing one anyway. wink

Unsolicited reviews are the future 😉 Thanks!

@schmitts
Copy link
Contributor

@schmitts the fix turned out to be almost trivial. Could you try out the fix_functions_namespace_cython branch with your code to confirm that it works correctly?

@mstimberg With the fix_functions_namespace_cython branch, the script works as expected also with the Cython backend.

@mstimberg
Copy link
Member Author

Great, thanks @schmitts for the confirmation. Going ahead with the merge now (ignore the failed test runs, it's due to GitHub stopping support for the outdated OS X image we used previously, I already corrected this in master).

@mstimberg mstimberg merged commit 222fde0 into master Jul 27, 2022
@mstimberg mstimberg deleted the fix_function_namespace_cython branch July 27, 2022 12:09
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.

Compilation error for SpatialNeuron, TimedArray and Cython
3 participants