Add infrastructure for generic input backends to feed devices #1456

sdiazpier · 2020-03-06T12:10:48Z

This PR adds the infrastructure required for the input part of NESTio.

See #1283 for the RecordingBackend counterpart of the NESTio infrastructure.

hakonsbm

Thank you for this great addition! I have quite a few suggestions, but there are a lot of similar ones, and all of them are minor.

models/spike_generator.cpp

models/spike_generator.h

nestkernel/input_backend_internal.cpp

nestkernel/input_backend_mpi.cpp

models/spike_generator.h

nestkernel/input_device.h

sdiazpier · 2020-04-01T07:40:11Z

Thank you dear @hakonsbm we will work on these corrections right away.

lionelkusch · 2020-04-02T14:07:55Z

Thank you @hakonsbm for all requests. @sdiazpier and me fix all of this.
If you have new requests don't hesitate to ask them.

hakonsbm

@sdiazpier @lionelkusch Thanks for the changes! Just a few more and I'm happy.

nestkernel/io_manager.cpp

nestkernel/recording_backend_mpi.cpp

nestkernel/input_backend.h

nestkernel/io_manager.h

nestkernel/input_backend_mpi.cpp

jougs

During NESTio/Output, all recorders were refactored to be derived from RecordingDevice instead of having one as a member. The base class RecordingDevice is now deriving from DeviceNode and Device.

During the work on this PR, a new base class InputDevice was introduced to handle the stimulus data coming from a backend instead of from parameter data members in the concrete stimulator class. I propose several changes:

A refactoring along the lines of the one described above would simplify the design. Namely, making StimulatingDevice a base class of the concrete stimulator.
To further simplify the design, the functionality of the new base class InputDevice should be included into the StimulatingDevice class.
The base class for recording backends was first called OutputBackend. We decided to rename them to RecordingBackends to make clear that they are not backends for general output operations from within NEST, but only handle the output generated by recorders. The same considerations apply to the naming of the current InputBackend base class, which IMO should be renamed to StimulatingBackend
The derivation of the SpikeParameters_ struct from the class Parameters_, which is a nested member of the InputDevice base class, looks unusual to me. I have not looked into this in detail, but think that a mechanism using get_/set_status() should be used instead and would result in a cleaner design. The same considerations apply to the State_ struct.
Why is the InputBackendInternal needed at all if it does nothing? Couldn't an empty input_from field just be interpreted in this way?
The InputBackend base class is currently more or less a copy of the RecordingBackend, which raises a number of questions:
- The documentation is still mostly describing the design and workings of the recording backend infrastructure with just the word "recording" replaced by "input".
- I don't understand how some of the interface functions that were taken from the recording backend design are actually useful for the input side.
  - One example is set_value_names(). If I understand the code correctly, all assignments of incoming data to device member variables is solely handled by update_from_backend() inside of the concrete stimulator and backends don't have to know about the data layout or field names at all.
  - I don't quite see how all the hooks are useful for the input side. In my view an input backend receives data from somewhere once per cycle and has to hand that over to all enrolled devices without the need for a more fine grained flow control. If I'm wrong, possible use cases should be explained in the documentation of the corresponding interface functions in the base class for input backends.
In general, some things should be renamed from my point of view:
- input_from ⟶ stimulus_source or data_source
- MPIFilePortsUnknown ⟶ MPIPortsFileUnknown
- The name of the function update_from_backend() is misleading, as the update() function of nodes in NEST is used to propagate their state once every time cycle. A better name would be set_data_from_input_backend() or something similar.

nestkernel/exceptions.h

nestkernel/input_backend_mpi.h

nestkernel/exceptions.cpp

nestkernel/exceptions.h

nestkernel/input_backend_mpi.cpp

models/spike_generator.cpp

lionelkusch · 2020-06-18T13:41:17Z

cc @maedoc

jougs · 2020-07-13T09:47:37Z

@lionelkusch, @sdiazpier: can you please give a short update of the status? Many thanks!

jougs · 2020-08-10T10:05:00Z

This is just a friendly ping!

sdiazpier · 2020-08-10T15:26:42Z

Dear @jougs we are still working on details merging the stimulating device with the input device. However, the more simple things we have mostly addressed already:

Making StimulatingDevice a base class of the concrete stimulator -> Done
To further simplify the design, the functionality of the new base class InputDevice should be included into the StimulatingDevice class.-> Problematic, in progress
The base class for recording backends was first called OutputBackend. We decided to rename them to RecordingBackends to make clear that they are not backends for general output operations from within NEST, but only handle the output generated by recorders. The same considerations apply to the naming of the current InputBackend base class, which IMO should be renamed to StimulatingBackend -> Done
Refactoring of SpikeParameters_ struct and State_ struct. -> Done
Why is the InputBackendInternal needed at all if it does nothing? Couldn't an empty input_from field just be interpreted in this way? -> In progress
The InputBackend base class is currently more or less a copy of the RecordingBackend, which raises a number of questions:
- The documentation is still mostly describing the design and workings of the recording backend infrastructure with just the word "recording" replaced by "input". -> Done
- I don't understand how some of the interface functions that were taken from the recording backend design are actually useful for the input side. -> Removed set_value_names() and unnecessary hooks.
Renaming:

input_from ⟶ stimulus_source or data_source -> Done to stimulus_source
MPIFilePortsUnknown ⟶ MPIPortsFileUnknown -> Done
Change name of function update_from_backend() to set_data_from_input_backend() -> Done to set_data_from_stimulating_backend()

As soon as we finalize the open points we will update the PR.

jougs · 2020-08-10T20:50:42Z

Awesome! Many thanks for the update and all the work in the background :-)

Co-authored-by: Jochen Martin Eppler <[email protected]>

…t_step_hook function

jougs

Thanks for your changes! Here are three more comments.

Pleas consider using the "batch commit" feature in the "Files Changed" pane in the future. That way you could bundle multiple suggestions into a single commit and also reduce the number of notification mails sent by GitHub. Thanks!

nestkernel/recording_backend_mpi.h

nestkernel/stimulating_backend.h

nestkernel/stimulating_backend_mpi.h

sdiazpier · 2021-05-17T15:49:35Z

Dear @jougs about the diagrams, communication_Nest.pdf, Nest_IO.pdf and state_Nest.pdf, where would it be good to store them so we can reference from the developer documentation?
I hope all the other comments are now solved :)

jougs · 2021-05-17T21:45:35Z

@sdiazpier, many thanks. All my suggestions are addressed!

Regarding the diagrams: @jessica-mitchell, what would be an appropriate location for such static content of the developer documentation? I tend to think that doc/devdoc/static would look nice alongside the already existing doc/userdoc/static. In any case, the Doxyfiles would have to be configured (and tested) accordingly, as this is the first time that we have custom files to be included in the developer documentation.

lionelkusch · 2021-05-18T07:54:24Z

Dear @jougs and @jessica-mitchell,
These are the image in png format to add to the Nest documentation :

jougs · 2021-05-21T00:17:31Z

I would appreciate if the figures were added also in a vector format or any source format they were generated from. The rationale for this is that we might need to tweak the figures in the future. I suggest to add them to doc/devdoc/static for the time being.

I would be OK with approving this without more extensive developer documentation, but would like to see at least some basic information about the idea on the user level stimulation guide. Also the links to the MPI backend documentation in the recording and stimulation guides are needed, so they can actually be found.

After reviewing #1929, I think we also need a section on the availability of the Stimulating Backend infrastructure in the NEST 2->3 guide. @jessica-mitchell, can you please give a hint on where this should best go?

jessica-mitchell · 2021-05-21T18:22:45Z

@jougs I'd suggest if it's a user level document to highlight the new feature, it should go here: doc/userdoc/guides/nest2_to_nest3/nest3_features/or put it in doc/userdoc/guides/ if it's much more in-depth and detailed. And include a link here doc/userdoc/guides/nest2_to_nest3/nest3_features/index.rst which also contains links to:

New functionality for node handles (neurons and devices)
New functionality for handling connections (synapses)
Parametrization
Simpler handling of random number generators
Recording from simulations
NEST Server

Also the devdoc/static sounds fine to me too

…nest-i

Add images for develloper documentation and add it in doxygen documentation. Add files from UMLET which generate the image. Modification of documentation of stimulus and recording with MPI backend for user documentation.

lionelkusch · 2021-06-03T13:37:41Z

Hi @jougs,
I include the images in the format of svg and integrate them in the developer documentation. I also include the xml file used by UMLET for generated these images.
I modify the documentation for including the modification of Nest 3 and the usage of this new backend, following the remarks.

@sdiazpier : Can you take a short look to check if I am not missing something?

sdiazpier · 2021-06-08T11:29:27Z

Dear @lionelkusch thanks so much for adding the figures and documentation. Dear @jougs I think we are ready for your final checks. Hopefully everything is now covered.

jougs · 2021-06-08T16:34:12Z

@lionelkusch, @sdiazpier: Many thanks!

I have just sent a PR towards your branch, in which I mostly reworked the structure of the user level documentation. I think you don't have to look into it in detail, as I already discussed most changes with @jessica-mitchell and @terhorstd. Once that's merged, I'll merge this one.

* Added epsilon to position checks Because of round-off errors when comparing node positions to lower left and upper right. * Removed unnecessary preprocessor directive * Added documentation on parameter comparators * Added test of round-off errors when connecting spatial populations * Added comments on epsilon values * Clarified a sentence * Fixed regex to correctly catch warnings * Updated return statements that are used in only rare cases * Added warnings from random123 library to known warnings * Fixed warning for implicitly declared operator=() * Fixed warning for unordered initialization of variables * Fixed warning for comparison of unsigned variable >= 0 * Fixed warnings from MUSIC models * Using a negative epsilon value for the other comparison as well * Apply suggestions from code review Co-authored-by: jessica-mitchell <[email protected]> * Apply suggestions from code review Co-authored-by: Hans Ekkehard Plesser <[email protected]> * Formatting * Updated return values in docstring * Add renamed 'time' to 'biological_time' kernel status variable * fix renamed 'time' to 'biological_time' kernel status variable * add colour highlighting to NEST 3.0 guide * fix rST syntax Co-authored-by: Håkon Bakke Mørk <[email protected]> * Rework stimulating device/backend documentation The purpose of this commit is to make the documentation of stimulating devices/backends more consistent with that of recording devices/backends. It also updates the corresponding guides. * Fix formatting * Fix formatting * Fix reordering warning Co-authored-by: Håkon Mørk <[email protected]> Co-authored-by: jessica-mitchell <[email protected]> Co-authored-by: Hans Ekkehard Plesser <[email protected]> Co-authored-by: clinssen <[email protected]> Co-authored-by: C.A.P. Linssen <[email protected]> Co-authored-by: Stine Brekke Vennemo <[email protected]>

sdiazpier · 2021-06-08T21:46:01Z

Dear @jougs and everyone involved in the PR, I just merged your changes to our branch. Thanks so much for the revised documentation.

jougs

Also many thanks from my side for all the hard work!

jougs changed the title ~~Nest I~~ Add infrastructure for generic input backends to feed devices Mar 10, 2020

jougs self-requested a review March 10, 2020 09:42

jougs added ZC: Kernel DO NOT USE THIS LABEL I: Behavior changes Introduces changes that produce different results for some users ZP: PR Created DO NOT USE THIS LABEL S: High Should be handled next T: Enhancement New functionality, model or documentation labels Mar 10, 2020

jougs added this to the NEST 3.0 milestone Mar 10, 2020

sdiazpier force-pushed the nest-i branch from 182daa4 to 6c5bf89 Compare March 10, 2020 11:43

terhorstd requested a review from hakonsbm March 16, 2020 10:55

hakonsbm suggested changes Mar 31, 2020

View reviewed changes

heplesser assigned sdiazpier Apr 7, 2020

heplesser removed ZC: Kernel DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL labels Apr 7, 2020

hakonsbm suggested changes Apr 7, 2020

View reviewed changes

nestkernel/io_manager.cpp Outdated Show resolved Hide resolved

nestkernel/recording_backend_mpi.cpp Outdated Show resolved Hide resolved

nestkernel/input_backend.h Outdated Show resolved Hide resolved

nestkernel/io_manager.h Show resolved Hide resolved

hakonsbm reviewed Apr 7, 2020

View reviewed changes

nestkernel/input_backend_mpi.cpp Outdated Show resolved Hide resolved

lionelkusch mentioned this pull request Apr 15, 2020

Tvb cosim the-virtual-brain/tvb-root#99

Closed

jougs requested changes Apr 21, 2020

View reviewed changes

jougs mentioned this pull request Apr 27, 2020

Cleanup and reformat user documentation for generators sarakonradi/nest-simulator#5

Merged

lionelkusch mentioned this pull request Jun 26, 2020

Epic : TVB Nest co-simulation multiscale (computer to supercomputer) multiscale-cosim/EBRAINS-cosim#21

Open

10 tasks

jougs mentioned this pull request Aug 10, 2020

[DESIGN] Refactoring the extension module system #1717

Open

9 tasks

sdiazpier and others added 8 commits May 17, 2021 11:56

Update nestkernel/recording_backend_mpi.h

c508cc2

Co-authored-by: Jochen Martin Eppler <[email protected]>

Update nestkernel/stimulating_backend_mpi.h

2c6f9ed

Co-authored-by: Jochen Martin Eppler <[email protected]>

Update nestkernel/recording_backend_mpi.h

f66d9db

Co-authored-by: Jochen Martin Eppler <[email protected]>

Update nestkernel/recording_backend_mpi.h

0bbf518

Co-authored-by: Jochen Martin Eppler <[email protected]>

Merge to master, addressed some documentation details and removed pos…

988b8b3

…t_step_hook function

Deleted call to post_step_hook from io manager

e567d2e

removed old random deviate

f357f21

Fix format

d12aac8

jougs reviewed May 17, 2021

View reviewed changes

nestkernel/recording_backend_mpi.h Outdated Show resolved Hide resolved

nestkernel/stimulating_backend.h Outdated Show resolved Hide resolved

nestkernel/stimulating_backend_mpi.h Show resolved Hide resolved

Solved some issues with documentation

9ffb9a3

lionelkusch added 3 commits June 2, 2021 12:13

Merge branch 'master' of https://github.com/nest/nest-simulator into …

abe745b

…nest-i

Documentation of MPI

3f8b344

Add images for develloper documentation and add it in doxygen documentation. Add files from UMLET which generate the image. Modification of documentation of stimulus and recording with MPI backend for user documentation.

Add the documentation for Nest_3 feature.

8f7594c

Small changes in documentation

5198a5d

jessica-mitchell mentioned this pull request Jun 8, 2021

Add a brief description of features of NEST 3.0 to its index #2061

Merged

Merge branch 'master' into nest-i

4a0f2a1

jougs approved these changes Jun 9, 2021

View reviewed changes

jougs merged commit 126616b into nest:master Jun 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add infrastructure for generic input backends to feed devices #1456

Add infrastructure for generic input backends to feed devices #1456

sdiazpier commented Mar 6, 2020 •

edited by jougs

Loading

hakonsbm left a comment

sdiazpier commented Apr 1, 2020

lionelkusch commented Apr 2, 2020

hakonsbm left a comment

jougs left a comment •

edited

Loading

lionelkusch commented Jun 18, 2020

jougs commented Jul 13, 2020

jougs commented Aug 10, 2020

sdiazpier commented Aug 10, 2020

jougs commented Aug 10, 2020

jougs left a comment

sdiazpier commented May 17, 2021

jougs commented May 17, 2021

lionelkusch commented May 18, 2021

jougs commented May 21, 2021

jessica-mitchell commented May 21, 2021

lionelkusch commented Jun 3, 2021

sdiazpier commented Jun 8, 2021

jougs commented Jun 8, 2021 •

edited

Loading

sdiazpier commented Jun 8, 2021

jougs left a comment

Add infrastructure for generic input backends to feed devices #1456

Add infrastructure for generic input backends to feed devices #1456

Conversation

sdiazpier commented Mar 6, 2020 • edited by jougs Loading

hakonsbm left a comment

Choose a reason for hiding this comment

sdiazpier commented Apr 1, 2020

lionelkusch commented Apr 2, 2020

hakonsbm left a comment

Choose a reason for hiding this comment

jougs left a comment • edited Loading

Choose a reason for hiding this comment

lionelkusch commented Jun 18, 2020

jougs commented Jul 13, 2020

jougs commented Aug 10, 2020

sdiazpier commented Aug 10, 2020

jougs commented Aug 10, 2020

jougs left a comment

Choose a reason for hiding this comment

sdiazpier commented May 17, 2021

jougs commented May 17, 2021

lionelkusch commented May 18, 2021

jougs commented May 21, 2021

jessica-mitchell commented May 21, 2021

lionelkusch commented Jun 3, 2021

sdiazpier commented Jun 8, 2021

jougs commented Jun 8, 2021 • edited Loading

sdiazpier commented Jun 8, 2021

jougs left a comment

Choose a reason for hiding this comment

sdiazpier commented Mar 6, 2020 •

edited by jougs

Loading

jougs left a comment •

edited

Loading

jougs commented Jun 8, 2021 •

edited

Loading