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 missing initialiser in copy constructor for stdp_nn_pre-centered_connection #1566

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

heplesser
Copy link
Contributor

This fixes #1435.

@jasperalbers This should also go into 2.20.1.

@heplesser heplesser added T: Bug Wrong statements in the code or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Apr 29, 2020
@heplesser heplesser added this to the NEST 2.20.1 milestone Apr 29, 2020
@heplesser heplesser requested review from stinebuu and clinssen April 29, 2020 12:04
@heplesser heplesser self-assigned this Apr 29, 2020
@clinssen
Copy link
Contributor

Great catch!

I am very disappointed that cppcheck does not catch this.

MSGBLD0155: [CPPC] [models/stdp_nn_pre-centered_connection.h:179]: (style) The function 'check_connection' is never used.
MSGBLD0155: [CPPC] [models/stdp_nn_pre-centered_connection.h:172]: (style) The function 'handles_test_event' is never used.
MSGBLD0155: [CPPC] [models/stdp_nn_pre-centered_connection.h:231]: (style) The function 'send' is never used.
MSGBLD0155: [CPPC] (information) Cppcheck cannot find all the include files (use --check-config for details)
MSGBLD0160: CPPCHECK for file ./models/stdp_nn_pre-centered_connection.h completed.

Do you suppose we should add Kplus member access to get_status() and set_status() as well, in the same model?

@heplesser
Copy link
Contributor Author

Do you suppose we should add Kplus member access to get_status() and set_status() as well, in the same model?

In principle yes, but I saw now that it is also inaccessible in, e.g., stdp_connection, so fixing this would be a bigger operation. Could you create a follow-up issue for that?

@heplesser
Copy link
Contributor Author

@Ziaeemehr Could you check if the fix in this PR solves the problem for you as well?

@Ziaeemehr
Copy link

@heplesser The PR solves the problem.

Phase 8: Running C++ tests (experimental)
-----------------------------------------
  Not running C++ tests because NEST was compiled without Boost.

NEST Testsuite Summary
----------------------
  NEST Executable: /home/abolfazl/prog/nest-build/bin/nest
  SLI Executable : /home/abolfazl/prog/nest-build/bin/sli
  Total number of tests: 870
     Passed: 860
     Skipped: 10 (9 PyNEST)
     Failed: 0 (0 PyNEST)

Built target installcheck

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: Bug Wrong statements in the code or documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Errors when trying to install NEST on Saga
4 participants