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

Add CMake flag to enable/disable hpc synapses #1049

Closed
wants to merge 2 commits into from

Conversation

jakobj
Copy link
Contributor

@jakobj jakobj commented Oct 15, 2018

under the assumption that most users do not use the hpc versions of the synapses, this PR adds a cmake option to enable/disable(default) hpc synapse models. this allows for more custom models to be registered before running out of possible synapse models. partly addresses #1043

@heplesser @suku248

@ikitayama
Copy link

ikitayama commented Oct 15, 2018 via email

@ikitayama
Copy link

ikitayama commented Oct 16, 2018 via email

@jakobj
Copy link
Contributor Author

jakobj commented Oct 16, 2018

thanks for the comment @ikitayama. i think this is a matter of taste. since most users will not use hpc synapses, and those who do, including you, know how to adjust cmake options to their desire, I thought it would be more reasonable to set the default to off, but I am open to discuss this.

@ikitayama
Copy link

@jakobj - I suppose a new CMake variable would be easily overlooked thus ended up a binary that is built to not load the HPC-themed models. And an HPC user might waist his/her couples of hours wondering at to why the models aren't loaded at run time. This is the scenario I worry about.

Given that we'd let many modules piled up in the code base, I'd like to oppose introducing module
load/unload switch in a ad-hoc fashion.

@stinebuu stinebuu added ZC: Installation DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL S: High Should be handled next T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Oct 25, 2018
@heplesser
Copy link
Contributor

@jakobj Would you consider closing this PR in favour of a long-term solution to #1043? See also discussion there.

@jakobj
Copy link
Contributor Author

jakobj commented Nov 16, 2018

closing, see #1043

@jakobj jakobj closed this Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Maintenance Work to keep up the quality of the code and documentation. ZC: Installation DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants