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.

Sign up for GitHub

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

Jump to bottom

Clean up the kernel in preparation for the module system refactoring #2208

Merged
merged 141 commits into from
Feb 14, 2022
Merged

Clean up the kernel in preparation for the module system refactoring #2208

merged 141 commits into from
Feb 14, 2022

Conversation

jougs
Copy link
Contributor

@jougs jougs commented Nov 12, 2021 •
edited

This PR is a bit of everything all over the place.

The main user-facing changes are summarized in the release notes file

To ease review of the many seemingly unconnected changes, here's a high-level list of changes contained:

  • Streamline the naming of model containers and access functions. Everything related to node models now has node_models in the name (used to be just models), everything connection model related has connection_model (used to be synapse_prototype or just prototype)
  • Make the SLI-level dictionaries modeldict, synapsedict, connruledict, growthcurvedict implementation details of the respective manages by providing proper access functions and removing all external references to them
  • Clean up and simplify the handling of the simulation resolution in SimulationManager and the setting of numbers of threads and virtual processes in VPManager
  • Make changing the number of threads less radical by introducing a change_number_of_threads() function in the manager interface that allows each manager to decide what to do, instead of calling finalize() and initialize() on each of them unconditionally
  • Clean up the registration and use of proxynodes and dummy_spike_sources and remove the related flag private_model from pertaining functions for registering models

jougs added 30 commits March 5, 2021 03:27
@jougs
Remove unused function
81bc709
@jougs
Minor cleanup along the way
f28080a
@jougs
Rename model vectors and use more sane names
994367c
@jougs
Minor code restructuring
2a45f03
@jougs
Replace the term synapse_prototype by connection_model
9cca7b5
@jougs
Replace the term synapse_model by connection_model
58b6399
@jougs
Remove unused functions from Model(Range)Manager
2c17ef0
@jougs
Minor cleanup for consistency and standards compliance
09c2da6
@jougs
Add asserts for everything checked in VPManager::set_status()
b7e94fc
@jougs
Remove unupdated part of copy-pasted comment
dcb00c1
@jougs
Update and extend comments of data structures for model prototypes
938a628
@jougs
Clean up and simplify checks for new numbers of threads and vps and a…
350c3dc 
…dapt pertaining regression test
@jougs
C++11-ify loops
b8645a1
@jougs
Simplify error handling
ad2d0c5
@jougs
Remove old syntax for making models available in SLI
c426843
@jougs
Remove ancient functions SaveModels and RestoreModels
6a31358
@jougs
Replace exception by assert, remove useless checks and now unused exc…
98e38d5 
…eption class
@jougs
Don't register proxynode as regular model and remove special code han…
f2086b0 
…dling that
@jougs
Remove mentions of NEST's modeldict from SLI language documentation
c8d1401
@jougs
Replace modeldict lookup by direct retrieval of model_id
c8d98e2
@jougs
Make node models available as node_models in kernel status dict inste…
b8a8c86 
…ad of via modeldict

This also adapts all pertaining example and test files, both SLI and PyNEST
From this point on, modeldict is merely used within the ModelManager
@jougs
Clarify comment wrt location of header file
3847a63
@jougs
Delete trailing whitespace
bd50ff8
@jougs
Replace terrible sentences by more pleasant ones
0ed9099
@jougs
Move imports below docstring
cfb5757
@jougs
Reformat comment
d3d7c5b
@jougs
Remove connruledict from user-facing interface, adapt documentation
d67fe53
@jougs
Update error messages to reflect current terminology in documentation
f9542c5
@jougs
Merge branch 'master' of github.com:nest/nest-simulator into refactor…
16590d3 
…_module_system
@jougs
Remove growthruledict from user-facing code
b375606
@jougs
Copy link
Contributor Author

jougs commented Jan 13, 2022

@heplesser, @hakonsbm: many thanks for your detailed reviews. I've addressed all of your points and would be grateful if you could quickly resolve conversations where possible.

!! Please do not merge yet! This should only go into 3.3 and we first need to get 3.2 out.

@jougs jougs changed the title Clean up the kernel in preparation for the module refactoring Clean up the kernel in preparation for the module system refactoring Jan 13, 2022
heplesser
heplesser approved these changes Jan 14, 2022
View reviewed changes
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@jougs Thanks a lot, especially for the re-instated and improved test! All fine now from my side.

hakonsbm
hakonsbm approved these changes Jan 14, 2022
View reviewed changes
Copy link
Contributor

@hakonsbm hakonsbm left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me now.

@heplesser
Copy link
Contributor

!! Please do not merge yet! This should only go into 3.3 and we first need to get 3.2 out.

Do you want to switch it to draft to avoid accidental merging?

@heplesser heplesser removed the request for review from stinebuu January 31, 2022 10:49
stinebuu
stinebuu approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@stinebuu stinebuu left a comment

Choose a reason for hiding this comment

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

Looks good to me once formatting/tests pass again.

Kernel automation moved this from PRs pending approval to PRs approved Jan 31, 2022
@jougs jougs mentioned this pull request Feb 14, 2022
Closed
@abigailm abigailm merged commit 6649606 into nest:master Feb 14, 2022
Kernel automation moved this from PRs approved to Done (PRs and issues) Feb 14, 2022
@jougs jougs mentioned this pull request Feb 23, 2022
Closed
@jougs jougs linked an issue Nov 28, 2022 that may be closed by this pull request
Dependency between nest.CopyModel and nest.Install #2287
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heplesser heplesser heplesser approved these changes

@hakonsbm hakonsbm hakonsbm approved these changes

@stinebuu stinebuu stinebuu approved these changes

Assignees

@jougs jougs

Labels
I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know S: High Should be handled next T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dependency between nest.CopyModel and nest.Install
5 participants
@jougs @heplesser @hakonsbm @stinebuu @abigailm