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

Fix issue #82: parrot neuron now emits multiple spikes via spike multiplicity instead of a loop #146

Merged
merged 5 commits into from
Nov 5, 2015

Conversation

flinz
Copy link
Contributor

@flinz flinz commented Nov 3, 2015

Addresses issue #82.
Added test for parrot neuron that checks for correct spike repetition with multiplicity.

commit b840820
Merge: 4668ea2 ce63c55
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Tue Nov 3 15:53:36 2015 +0100

    Merge branch 'master' into issue-parrot_neuron-multiplicity

    * master:

commit 4668ea2
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Tue Nov 3 15:29:13 2015 +0100

    Updated tests to conform to changes in parrot-multipart branch;

commit 1b8ddac
Merge: bce4bf9 9c4c2f3
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Tue Nov 3 15:27:01 2015 +0100

    Merge branch 'parrot-multiport' into issue-parrot_neuron-multiplicity

    * parrot-multiport:
      removed unnecessary variables gid_source and gid_parrot;

commit bce4bf9
Merge: ef31ab0 3d8da20
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Tue Nov 3 13:20:45 2015 +0100

    Merge branch 'parrot-multiport' into issue-parrot_neuron-multiplicity

    * parrot-multiport:
      Added tests for STDP protocols between parrot neurons to potentiate and depress connection; Accounted for delays in simulation times; Addresses https://github.com/nest/nest-simulator/pull/118/files#r43736299 and https://github.com/nest/nest-simulator/pull/118/files#r43702948; Fixes https://github.com/nest/nest-simulator/pull/118/files#r43702538 and nest#118 (comment);
      Import GetStatus/SetStatus from the info component, where they were moved in a3ec4d4.
      Fix the stop condition of process_static_analysis.
      Added explanation for recursive import code.
      Moved Get/SetStatus from node to info component of HL API.
      Added information on steps to execute after creating the release tarball.
      Validate parameter values in setters of tsodyks synapses
      Added empty __init__.py to make the lib directory a module.
      Split Python high-level API into multiple files and made loading of the submodules dynamic
      Tsodyks synapse model with common properties

commit ef31ab0
Merge: ca70656 9425619
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Tue Nov 3 10:19:25 2015 +0100

    Merge branch 'parrot-multiport' into issue-parrot_neuron-multiplicity

    * parrot-multiport:

commit ca70656
Merge: 48a9feb c80a8be
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Tue Nov 3 10:18:11 2015 +0100

    Merge branch 'master' into issue-parrot_neuron-multiplicity

    * master:
      Apply @lekshmideepu suggestions
      Filter out empty entries in changed files
      some cleanup
      Modify travis.yml
      Implemented two fixes for the K computer:
      Print status about uploading to S3
      Remove cppcheck files after static analysis.
      Adjust comment and have starting and ending line at summary
      Correct copyright header.
      Add a script to parse travis log and give a nice summary.
      Removed superfluous and outdated files

commit 48a9feb
Merge: 0b96cd1 4d939e5
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Mon Oct 19 10:14:01 2015 +0200

    Merge branch 'parrot-multiport' into issue-parrot_neuron-multiplicity

    * parrot-multiport:
      Added superclass (archiving_node) get_status to parrot_neuron;

commit 0b96cd1
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Fri Oct 16 16:08:11 2015 +0200

    added test for parrot neuron multiplicity.

commit ff0cb6d
Author: Alex Seeholzer <seeholzer@gmail.com>
Date:   Fri Oct 16 15:39:45 2015 +0200

    Changed parrot neuron to emit multiple spikes via spike multiplicity instead of a loop; Addresses nest#82
@flinz flinz changed the title Changed parrot neuron to emit multiple spikes via spike multiplicity instead of a loop Fix issue https://github.com/nest/nest-simulator/issues/82: parrot neuron now emits multiple spikes via spike multiplicity instead of a loop Nov 3, 2015
@flinz flinz changed the title Fix issue https://github.com/nest/nest-simulator/issues/82: parrot neuron now emits multiple spikes via spike multiplicity instead of a loop Fix issue #82: parrot neuron now emits multiple spikes via spike multiplicity instead of a loop Nov 3, 2015
@heplesser
Copy link
Contributor

Looks good, I created a PR to @flinz repo with improved documentation and one more test.

SpikeEvent se;
se.set_multiplicity( current_spikes_n );

network()->send( *this, se, lag );
set_spiketime( Time::step( origin.get_steps() + lag + 1 ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

I need to double-check whether set_spiketime() needs to be called once per outgoing spike, i.e., as many times as multiplicity indicates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#77 (comment) seems to suggest that this is indeed the case.

this would result in

for (ulong_t i=0;  i<current_spikes_n; i++ )
    set_spiketime( Time::step( origin.get_steps() + lag + 1 ) );

with which we're almost back to the old form -- although it's a little cleaner with the handling of multiplicity.

It should be noted that the original model did not set the spike_times multiply -- but the parrot probably wasn't used for plasticity much yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that you need this. Can you please add this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

@flinz I agree with @jougs and after more meditation over the spike archiving and plasticity (see #77) think that we do have to call set_spiketime() once for every spike. One this PR is merged, I will extend set_spiketime() to accept multiplicity as argument in a separate PR, but I don't want to delay this PR further.

…-squashed

Revised documentation of parrot_neuron and added one more test.
No parameters to be set in the status dictionary.

References:
No references

Author: May 2006, Reichert, Morrison
Copy link
Contributor

Choose a reason for hiding this comment

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

This should now be

Author: David Reichert, Abigail Morrison, Alexander Seeholzer, Hans Ekkehard Plesser
FirstVersion: May 2006

@jougs
Copy link
Contributor

jougs commented Nov 4, 2015

Very nice work. I'm all 👍 once my comments above have been addressed.

@heplesser
Copy link
Contributor

👍 once the loop around set_multiplicity() is in place and Jochen's comments are addressed.

* master:
  Use NewConnect instead of DivergentConnect and remove non-default weight in favor of higher rate.
  Fixed error message and reporting in case CSA is not available.
  Make wording more consistent with respect to active and passive speech.
  improvements as discussed in PR nest#127
  Added reference
  Updated CSA examples to new documentation guidelines.
  Replace NAN, std::isnan by numerics::nan, numerics::isnan
jougs added a commit that referenced this pull request Nov 5, 2015
…quashed

Fix issue #82: parrot neuron now emits multiple spikes via spike multiplicity instead of a loop
@jougs jougs merged commit ca0513f into nest:master Nov 5, 2015
@flinz flinz deleted the issue-parrot_neuron-multiplicity-squashed branch November 6, 2015 11:57
tammoippen added a commit to tammoippen/nest-simulator that referenced this pull request Jan 7, 2016
# Conflicts:
#	models/parrot_neuron.cpp
@gewaltig gewaltig mentioned this pull request Jan 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants