Document kernel status dictionary entries #1373

morales-gregorio · 2019-12-20T11:35:52Z

This is the PR trying to solve Issue #1350

I have looked into nest.help("kernel") and I found a few issues.

initial_connector_capacity, large_connector_limit, large_connector_growth_factor are present in the help documentation but they do not seem to be present in the kernel status dictionary from NEST >=2.14. I have removed them from the list.
grng_seed and rng_seed are specified as write only, but the GetKernelStatus() does return them, they can also be read (I removed the write only tag from their description).
There is a long list of keys that are present in the kernel dictionary (NEST 2.18.0) and not documented in nest.help("kernel"):

 ['adaptive_spike_buffers',
 'adaptive_target_buffers',
 'buffer_size_secondary_events',
 'buffer_size_spike_data',
 'buffer_size_target_data',
 'growth_factor_buffer_spike_data',
 'growth_factor_buffer_target_data',
 'keep_source_table',
 'local_spike_counter',
 'max_buffer_size_spike_data',
 'max_buffer_size_target_data',
 'max_num_syn_models',
 'sort_connections_by_source',
 'structural_plasticity_synapses',
 'structural_plasticity_update_interval',
 'time_collocate',
 'time_communicate']

I have added them, their types and whether they are read only or can be read and written. However their descriptions should be filled before this PR can be merged to master. (Marked the descriptions with TODO flags)

Best,
Aitor

jakobj · 2020-02-12T17:47:10Z

great, thanks @morales-gregorio! here are some descriptions of the entries marked with TODO, partly extracted directly from the source files:

adaptive_spike_buffers: whether MPI buffers for communication of spikes resize on the fly
adaptive_target_buffers: whether MPI buffers for communication of connections resize on the fly
buffer_size_spike_data: total size of MPI buffer for communication of spikes
buffer_size_target_data: total size of MPI buffer for communication of connections
growth_factor_buffer_spike_data: if MPI buffers for communication of spikes resize on the fly, grow them by this factor each round
growth_factor_buffer_target_data :if MPI buffers for communication of connections resize on the fly, grow them by this factor each round
keep_source_table : whether to keep source table after connection setup is complete
max_buffer_size_spike_data : maximal size of MPI buffers for communication of spikes
max_buffer_size_target_data : maximal size of MPI buffers for communication of connections
max_num_syn_models : maximal number of synapse models supported
sort_connections_by_source : whether to sort connections by their source; increases construction time of presynaptic data structures, decreases simulation time if the average number of outgoing connections per neuron is smaller than the total number of threads

morales-gregorio · 2020-02-13T16:06:54Z

Thanks @jakobj for the collection efforts! I have included those to the PR. Now only the descriptions for:

buffer_size_secondary_events
local_spike_counter
structural_plasticity_synapses
structural_plasticity_update_interval
time_collocate
time_communicate

are missing. Any suggestions on where such descriptions can be found? or who may know?

stinebuu · 2020-02-26T13:51:14Z

@morales-gregorio I think something happened when you merged your branch with master, your PR now contains 1185 changed files :)

When it comes to local_spike_counter it is the number of generated spike events during simulation.

heplesser · 2020-03-17T14:15:08Z

@morales-gregorio I am sorry I did not get around to reviewing your PR before the NEST-3 merger. As @stinebuu pointed out, something seems to have gone wrong when you merged a recent master into your PR. Could you try to either do this merge once more, or create a new PR, adding the new documentation to the current master? I will be fast in the next round!

morales-gregorio · 2020-03-24T09:15:24Z

Hey @heplesser @stinebuu Thanks for having a look. Something went very wrong with my PR indeed, I will look into that possibly creating a new PR.
However, before anything can be merged I still need definitions for these keywords:

buffer_size_secondary_events
local_spike_counter
structural_plasticity_synapses
structural_plasticity_update_interval
time_collocate
time_communicate

Do you know what they are and what they do? Or do you know who knows?
Thanks!

heplesser · 2020-03-24T10:11:58Z

buffer_size_secondary_events

Size of MPI buffers for communicating secondary events (in bytes, per MPI rank, for developers)

local_spike_counter

Number of spikes fired by neurons on a given MPI rank since NEST was started or the last ResetKernel. Only spikes from "normal" neurons (neuron models with proxies) are counted, not spikes generated by devices such as poisson_generator.

@sdiazpier Could you explain the next two

structural_plasticity_synapses
structural_plasticity_update_interval

@suku248 or @jarsi Could you explain these two:

time_collocate
time_communicate

sdiazpier · 2020-03-24T10:31:25Z

structural_plasticity_synapses
Is a dictionary which defines all synapses which are plastic for the structural plasticity algorithm. Each entry in the dictionary is composed of a synapse model, the pre synaptic element and the post synaptic element.
structural_plasticity_update_interval
Defines the time interval in ms at which the structural plasticity manager will make changes in the structure of the network (creation and deletion of plastic synapses).

jarsi · 2020-03-24T13:25:11Z

The keywords

time_collocate
time_communicate

should log the time spent collocating and communicating. It seems that these variables have lost their purpose and they do not seem to properly function anymore. From my experiments I know that they worked with the 4g Kernel. If I remember correctly, they do not work with current master anymore.

We recently had a conference on a basic instrumentation of NEST. The results have been gathered in issue #1471. These two variables were also part of the discussion. Most probable both are obsolete and will be replaced with something else.

morales-gregorio · 2020-03-24T13:41:02Z

@sdiazpier @jarsi thanks for your responses!

I could omit time_collocate and time_communicate from the list of parameters in the documentation. Or I could also annotate them with the descriptions from issue #1471 and note that these are now deprecated.

Whatever is preferrable, what do you think?

heplesser · 2020-03-31T13:47:38Z

I could omit time_collocate and time_communicate from the list of parameters in the documentation. Or I could also annotate them with the descriptions from issue #1471 and note that these are now deprecated.

Whatever is preferrable, what do you think?

I would omit them now. Once #1471 brings a properly defined set of timings to the table, the PR can add the documentation. We need to keep dependencies between PRs and issues down ;).

morales-gregorio · 2020-04-02T12:19:41Z

Hi!

I have reset to the original fork master (which has temporarily closed this PR), pulled from the upstream master and I now pushed one commit with all the documented parameters into it.

Please have a look if you think there is something wrong. I have ignored time_collocate and time_communicate as suggested. I have also added all other descriptions as provided by the different people, such that no parameter is missing a description.

Best,
Aitor

heplesser

@morales-gregorio I think we should not duplicate this large amount of documentation in SetKS and GetKS. We will not manage to keep it in sync. I am not decided yet if I would place the doc with Get or Set: In Get, all parameters apply, but in Set the documentation is most needed. So maybe Set is the best place. As default I would assume all params to be read/write, and mark only those that are read-only explicitly. Given the very long list of parameters, I would suggest grouping them with sub-headings.

@sarakonradi I'd much appreciate your input.

heplesser · 2020-04-02T14:18:15Z

pynest/nest/lib/hl_api_simulation.py

+    resolution : double
+        The resolution of the simulation (in ms)
+    time : double
+        The current simulation time


You should either give units for all quantities or for none.

Hey @heplesser thanks for spotting this. I would argue in favor of providing units always. But I am not sure which units are needed in each case.

Is it always milliseconds for time-related keywords? What about the rest?

Good point! In the kernel, all time-related keywords are in ms. The only other quantities with units I spot are potentially the various buffer sizes, although they may also be number of buffer elements, not size in bytes (@suku248 @jakobj could you help on this?); and the wfr_tol, which might be relative or absolute (@janhahne @ddahmen @rgutzen could you provide details?).

@heplesser wft_tol is the absolute tolerance for the iterative scheme to stop (every membrane potential/rate change from one iteration to the next needs to be below this value for all neurons that use waveform relaxation).

@janhahne So this means that if I simulate a network of spiking neurons with gap junctions, wfr_tol would have units of mV, because it refers to membrane potential, and if I simulate a network of rate neurons it would have units of spikes/second?

@heplesser yes, exactly :)

sarakonradi

@morales-gregorio Thanks a lot for your work! Just added two minor comments. I agree with @heplesser about grouping the parameters into sublists.

@jhnnsnk If you have anything to add regarding print_time, please let us know.

sarakonradi · 2020-04-03T08:05:05Z

pynest/nest/lib/hl_api_simulation.py

+        defines all synapses which are plastic for the structural plasticity
+        algorithm. Each entry in the dictionary is composed of a synapse model,
+        the pre synaptic element and the post synaptic element. (read/write)


Shouldn't this be Defines the time interval in ms at which the structural plasticity manager will make changes in the structure of the network (creation and deletion of plastic synapses).?

Thanks for spotting this one! Yes indeed

sarakonradi · 2020-04-03T08:11:11Z

pynest/nest/lib/hl_api_simulation.py

+         If MPI buffers for communication of spikes resize on the fly, grow
+         them by this factor each round (read/write)
+    growth_factor_buffer_target_data : float
+        if MPI buffers for communication of connections resize on the fly, grow


I suggest capitalizing the first letter of every definition for consistency.

terhorstd · 2020-04-03T15:49:25Z

pynest/nest/lib/hl_api_simulation.py

+         If MPI buffers for communication of spikes resize on the fly, grow
+         them by this factor each round (read/write)
+    growth_factor_buffer_target_data : float
+        if MPI buffers for communication of connections resize on the fly, grow


Suggested change

if MPI buffers for communication of connections resize on the fly, grow

If MPI buffers for communication of connections resize on the fly, grow

terhorstd · 2020-04-03T15:49:41Z

pynest/nest/lib/hl_api_simulation.py

+    max_num_syn_models : int
+        Maximal number of synapse models supported (read only)
+    sort_connections_by_source : bool
+        whether to sort connections by their source; increases construction


Suggested change

whether to sort connections by their source; increases construction

Whether to sort connections by their source; increases construction

sarakonradi · 2020-04-22T13:22:06Z

@morales-gregorio Thanks for the update! It looks like something went wrong after merging master, since all the files that were changed in other PRs appear under "Files changed" now. Perhaps it is best to undo the last commit and merge master again?

…rs. Incuding all parameters discussed in the PR. Closes nest#1350

… SetKernelStatus

…cumentation structure, remove (read/write) flags, capitalize descriptions, rewrite double --> float

morales-gregorio · 2020-04-23T09:02:38Z

Hey @sarakonradi I think that the faulty history is now fixed, I figured out that using rebase is a way better idea than merging master to my branch.

sarakonradi

Hi @morales-gregorio, thanks for fixing the history! Overall, it looks good to me. I replied to @heplesser's comment regarding Other parameters inline.

Talking with @sarakonradi we noticed that most other functions in this file (an potentially most others) do not follow the standard for the headings (i.e. have no blank line after the heading).
Maybe a future PR could handle that? It would have to be rather large one, making changes to the docstring of most functions, but could be used to establish the standard for the documentation.

Yes, definitely. We should examine the formatting of all functions in a different PR later on.

sarakonradi · 2020-04-23T12:11:56Z

pynest/nest/lib/hl_api_simulation.py

@@ -190,99 +190,134 @@ def SetKernelStatus(params):

    Other parameters


I agree with @heplesser regarding Other parameters and suggest changing it toParams dictionary or Parameter dictionary.

morales-gregorio · 2020-04-23T12:30:05Z

@sarakonradi @heplesser I just went ahead and changed Other parameters -> Params dictionary.
Which I believe addresses all review comments

sarakonradi

Great!

terhorstd added this to the NEST 3.0 milestone Jan 6, 2020

terhorstd requested review from heplesser and hakonsbm January 6, 2020 11:23

heplesser mentioned this pull request Jan 14, 2020

Include structural plasticity status information in kernel status #777

Closed

morales-gregorio closed this Apr 2, 2020

morales-gregorio force-pushed the doc_kernelstatus branch from fa5a09e to 00e0d3c Compare April 2, 2020 12:09

morales-gregorio reopened this Apr 2, 2020

heplesser requested changes Apr 2, 2020

View reviewed changes

heplesser requested review from sarakonradi and removed request for hakonsbm April 2, 2020 14:25

sarakonradi suggested changes Apr 3, 2020

View reviewed changes

sarakonradi mentioned this pull request Apr 3, 2020

Final checks of documentation #1445

Closed

24 tasks

terhorstd reviewed Apr 3, 2020

View reviewed changes

diesmann mentioned this pull request Apr 22, 2020

Change definition of realtime factor in print_time #1491

Merged

heplesser requested review from sarakonradi and heplesser April 22, 2020 12:43

morales-gregorio force-pushed the doc_kernelstatus branch from 8725bfc to c190897 Compare April 23, 2020 08:52

morales-gregorio added 15 commits April 23, 2020 10:59

Update documentation for GetKernelStatus and SetKernelStatus paramete…

0381001

…rs. Incuding all parameters discussed in the PR. Closes nest#1350

remove parameter list from GetKernelStatus in favor of a full list in…

9e20af8

… SetKernelStatus

fix typo spotted by @sarakonradi

04a49dc

Add all keys to SetKernelStatus, change notes according to the new do…

41e4ec3

…cumentation structure, remove (read/write) flags, capitalize descriptions, rewrite double --> float

move (read only) flag next to the type, may improve readability

ebd41b0

Reorganize key values into different sections

2c46652

make local_num_threads writeable

d909bc0

indicate that off_grid_spiking is writeable

5c08b50

indicate that num_processes is read only

752a934

correct spelling

01b55a6

should not -> cannot

098482f

internally calculated -> set by the NEST kernel

4002476

Remove notes and update description under other parameters heading

689a96d

fix possible typo

baa2fe2

Change header and subheader format

1ad995b

morales-gregorio force-pushed the doc_kernelstatus branch from c190897 to 1ad995b Compare April 23, 2020 09:01

sarakonradi reviewed Apr 23, 2020

View reviewed changes

Other parameters -> Params dictionary

106098c

sarakonradi approved these changes Apr 23, 2020

View reviewed changes

heplesser approved these changes Apr 23, 2020

View reviewed changes

heplesser merged commit 1d10715 into nest:master Apr 23, 2020

terhorstd changed the title ~~Doc kernelstatus~~ Document kernel status dictionary entries Jun 10, 2021

morales-gregorio mentioned this pull request Jun 30, 2021

Check validity of parameters given to SetKernelStatus #2089

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document kernel status dictionary entries #1373

Document kernel status dictionary entries #1373

morales-gregorio commented Dec 20, 2019

jakobj commented Feb 12, 2020

morales-gregorio commented Feb 13, 2020 •

edited

Loading

stinebuu commented Feb 26, 2020

heplesser commented Mar 17, 2020

morales-gregorio commented Mar 24, 2020

heplesser commented Mar 24, 2020 •

edited

Loading

sdiazpier commented Mar 24, 2020

jarsi commented Mar 24, 2020

morales-gregorio commented Mar 24, 2020

heplesser commented Mar 31, 2020

morales-gregorio commented Apr 2, 2020

heplesser left a comment

heplesser Apr 2, 2020

morales-gregorio Apr 14, 2020

heplesser Apr 15, 2020

janhahne Apr 15, 2020

heplesser Apr 15, 2020

janhahne Apr 16, 2020

sarakonradi left a comment

sarakonradi Apr 3, 2020

morales-gregorio Apr 14, 2020

sarakonradi Apr 3, 2020

terhorstd Apr 3, 2020

terhorstd Apr 3, 2020

sarakonradi commented Apr 22, 2020

morales-gregorio commented Apr 23, 2020

sarakonradi left a comment

sarakonradi Apr 23, 2020

morales-gregorio commented Apr 23, 2020

sarakonradi left a comment

	if MPI buffers for communication of connections resize on the fly, grow
	If MPI buffers for communication of connections resize on the fly, grow

	whether to sort connections by their source; increases construction
	Whether to sort connections by their source; increases construction

		@@ -190,99 +190,134 @@ def SetKernelStatus(params):

		Other parameters

Document kernel status dictionary entries #1373

Document kernel status dictionary entries #1373

Conversation

morales-gregorio commented Dec 20, 2019

jakobj commented Feb 12, 2020

morales-gregorio commented Feb 13, 2020 • edited Loading

stinebuu commented Feb 26, 2020

heplesser commented Mar 17, 2020

morales-gregorio commented Mar 24, 2020

heplesser commented Mar 24, 2020 • edited Loading

sdiazpier commented Mar 24, 2020

jarsi commented Mar 24, 2020

morales-gregorio commented Mar 24, 2020

heplesser commented Mar 31, 2020

morales-gregorio commented Apr 2, 2020

heplesser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarakonradi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarakonradi commented Apr 22, 2020

morales-gregorio commented Apr 23, 2020

sarakonradi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morales-gregorio commented Apr 23, 2020

sarakonradi left a comment

Choose a reason for hiding this comment

morales-gregorio commented Feb 13, 2020 •

edited

Loading

heplesser commented Mar 24, 2020 •

edited

Loading