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

Adding inhomogeneous poisson generator model #671

Merged
merged 15 commits into from Mar 10, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions models/CMakeLists.txt
Expand Up @@ -70,6 +70,7 @@ set( models_sources
music_message_in_proxy.h music_message_in_proxy.cpp
noise_generator.h noise_generator.cpp
parrot_neuron.h parrot_neuron.cpp
inh_poisson_generator.h inh_poisson_generator.cpp
poisson_generator.h poisson_generator.cpp
pp_psc_delta.h pp_psc_delta.cpp
pp_pop_psc_delta.h pp_pop_psc_delta.cpp
Expand Down
213 changes: 213 additions & 0 deletions models/inh_poisson_generator.cpp
@@ -0,0 +1,213 @@
/*
* inh_poisson_generator.cpp
*
* This file is part of NEST.
*
* Copyright (C) 2004 The NEST Initiative
*
* NEST is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2 of the License, or
* (at your option) any later version.
*
* NEST is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with NEST. If not, see <http://www.gnu.org/licenses/>.
*
*/
#include "inh_poisson_generator.h"

#include "event_delivery_manager_impl.h"
#include "kernel_manager.h"

#include "dict.h"
#include "dictutils.h"
#include "doubledatum.h"

#include "exceptions.h"
#include "integerdatum.h"
#include "arraydatum.h"
#include "numerics.h"
#include "universal_data_logger_impl.h"

#include <cmath>
#include <limits>

/* ----------------------------------------------------------------
* Default constructors defining default parameter
* ---------------------------------------------------------------- */

nest::inh_poisson_generator::Parameters_::Parameters_()
: rate_times_() // ms
, rate_values_() // spikes/ms
{
}

/* ----------------------------------------------------------------
* Parameter extraction and manipulation functions
* ---------------------------------------------------------------- */

void
nest::inh_poisson_generator::Parameters_::get( DictionaryDatum& d ) const
{
( *d )[ names::rate_times ] =
DoubleVectorDatum( new std::vector< double_t >( rate_times_ ) );
( *d )[ names::rate_values ] =
DoubleVectorDatum( new std::vector< double_t >( rate_values_ ) );
}

void
nest::inh_poisson_generator::Parameters_::set( const DictionaryDatum& d,
Buffers_& b )
{
const bool ut =
updateValue< std::vector< double_t > >( d, names::rate_times, rate_times_ );
const bool uv = updateValue< std::vector< double_t > >(
d, names::rate_values, rate_values_ );
Copy link
Contributor

Choose a reason for hiding this comment

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

ut and uv are rather too short and uninformative.

Copy link
Author

Choose a reason for hiding this comment

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

done


if ( ut xor uv )
{
throw BadProperty( "Rate times and values must be reset together." );
}

if ( rate_times_.size() != rate_values_.size() )
{
throw BadProperty( "Rate times and values have to be the same size." );
}

// ensure amp times are strictly monotonically increasing
if ( rate_times_.empty() == false )
Copy link
Contributor

Choose a reason for hiding this comment

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

if ( not rate_times_.empty() )

Copy link
Author

Choose a reason for hiding this comment

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

done

{
std::vector< double_t >::const_iterator prev = rate_times_.begin();
std::vector< double_t >::const_iterator next = prev + 1;
for ( ; next != rate_times_.end(); ++next, ++prev )
{
if ( *prev >= *next )
{
throw BadProperty( "Rate times must strictly increasing." );
}
}
}

if ( ut && uv )
{
b.idx_ = 0; // reset if we got new data
}
}

/* ----------------------------------------------------------------
* Default and copy constructor for node
* ---------------------------------------------------------------- */

nest::inh_poisson_generator::inh_poisson_generator()
: Node()
, device_()
, P_()
{
}

nest::inh_poisson_generator::inh_poisson_generator(
const inh_poisson_generator& n )
: Node( n )
, device_( n.device_ )
, P_( n.P_ )
{
}


/* ----------------------------------------------------------------
* Node initialization functions
* ---------------------------------------------------------------- */
void
nest::inh_poisson_generator::init_state_( const Node& proto )
{
const inh_poisson_generator& pr = downcast< inh_poisson_generator >( proto );

device_.init_state( pr.device_ );
}

void
nest::inh_poisson_generator::init_buffers_()
{
device_.init_buffers();
B_.idx_ = 0;
B_.rate_ = 0;
}

void
nest::inh_poisson_generator::calibrate()
{
device_.calibrate();
V_.h_ = Time::get_resolution().get_ms();
}

/* ----------------------------------------------------------------
* Update function and event hook
* ---------------------------------------------------------------- */

void
nest::inh_poisson_generator::update( Time const& origin,
const long from,
const long to )
{
assert(
to >= 0 && ( delay ) from < kernel().connection_manager.get_min_delay() );
assert( from < to );
assert( P_.rate_times_.size() == P_.rate_values_.size() );

const long t0 = origin.get_steps();

// random number generator
librandom::RngPtr rng = kernel().rng_manager.get_rng( get_thread() );


Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty line.

Copy link
Author

Choose a reason for hiding this comment

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

done

// Skip any times in the past. Since we must send events proactively,
// idx_ must point to times in the future.
const long first = t0 + from;
while ( B_.idx_ < P_.rate_times_.size()
&& Time( Time::ms( P_.rate_times_[ B_.idx_ ] ) ).get_steps() <= first )
{
++B_.idx_;
}

for ( long offs = from; offs < to; ++offs )
{
const long curr_time = t0 + offs;

// Keep the amplitude up-to-date at all times.
// We need to change the amplitude one step ahead of time, see comment
// on class SimulatingDevice.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is a typo, should read StimulatingDevice right?

Copy link
Author

Choose a reason for hiding this comment

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

I think so, changed.

if ( B_.idx_ < P_.rate_times_.size()
&& curr_time + 1
== Time( Time::ms( P_.rate_times_[ B_.idx_ ] ) ).get_steps() )
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen here if the rate changes twice within a time step? Since you can only increase B_.idx_ by one for each value of offs, you would probably get out of step for the remainder of the simulation.

Note that this is a deep problem: which rate should you use if the rate changes multiple times within an interval. Indeed, the question is also how to handle rate changes inside an interval correctly. This is especially critical to ensure consistent results when simulating the same network with different resolutions. The sinusoidal_poisson_generator always uses the rate at the end of the time step, so a consistent solution would be to always use the rate valid at then end of the step for the entire step. This needs to be documented, and you need code here that skips far enough.

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 to @heplesser, you will get out of step if the rate_times are not on the time-grid of the simulation. For example you will increase B_.idx_ only once within the dmin loop, if you specify the rate_times with a time-interval equal to half of the simulation time-step. I anyway suggest that the user can only specify rate_times which lie on the simulation time grid. However, the simulation time grid can change after the device has been specified and parameters have already been given. Then a warning should be raised. @heplesser, do you have an idea how to handle this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jschuecker NEST prohibits changes in resolution after the first network node has been created, so resolution changes are not an immediate problem. Simulating the same network at different resolutions may lead to different rate profiles, though, unless all times at which the rates change are multiples of all resolutions used.

Forcing the times of rate changes onto the resolution time grid is a sensible idea. We do this in spike_generator, see its Parameters::set() method for an implementation. That model also shows how to optionally disable the grid-forcing.

Choose a reason for hiding this comment

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

So far, when using this model, we have always made sure that all the rate_times are multiples of the simulation resolution. I guess the simplest way to address this issue would be to raise an error if this is not the case, or, as @heplesser suggests, force the rate_times onto the time grid (issuing a warning, so that the user knows the chosen rate_times will be changed)...

Copy link
Contributor

Choose a reason for hiding this comment

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

When writing code for a widely used simulator, one always needs to keep an eye on protecting users from doing things with untoward consequences. I would suggest to implement the grid-forcing as in spike_generator. That would also provide consistency between these two generators that can be "fed" arrays of times.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the long delay, will try to implement all requests / suggestions as soon as possible.. @heplesser should grid enforcement be optional as in spike_generator, controlled via the precise_times and allow_offgrid_times flags or not?

{
B_.rate_ = P_.rate_values_[ B_.idx_ ] / 1000.0; // scale the rate to ms^-1
B_.idx_++;
Copy link
Contributor

Choose a reason for hiding this comment

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

++B_.idx_;, we generally prefer prefix operators since they are faster for complex data types.

Copy link
Author

Choose a reason for hiding this comment

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

done

}

// create spikes
if ( B_.rate_ > 0 && device_.is_active( Time::step( t0 + offs ) ) )
{
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of t0 + offs you could use curr_time from line 179.

Copy link
Author

Choose a reason for hiding this comment

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

done

DSSpikeEvent se;
kernel().event_delivery_manager.send( *this, se, offs );
Copy link
Contributor

Choose a reason for hiding this comment

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

This code sends different spike trains to each target. To send the same train to all targets, you need additional code, see, e.g., sinusoidal_poisson_generator.

Choose a reason for hiding this comment

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

Should this be included? It is generally desirable that each target receives an independent realization..

Copy link
Contributor

Choose a reason for hiding this comment

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

@rcfduarte Sending the same train to all targets is not strictly necessary. One can always achieve the same effect by connecting the generator to a single parrot_neuron and then connecting that to all neurons that should receive identical spike trains. It was just a suggestion, since we have this feature in sinusoidal_{poisson,gamma}_generator. The reason for implementing it there was that we use those generators often to represent retinal ganglion cells, i.e., individual neurons, not background populations projecting noise. But if that use case is not relevant for you, there is not need to add support for it.

}
}
}

void
nest::inh_poisson_generator::event_hook( DSSpikeEvent& e )
{
librandom::RngPtr rng = kernel().rng_manager.get_rng( get_thread() );
V_.poisson_dev_.set_lambda( B_.rate_ * V_.h_ );
long n_spikes = V_.poisson_dev_.ldev( rng );

if ( n_spikes > 0 ) // we must not send events with multiplicity 0
{
e.set_multiplicity( n_spikes );
e.get_receiver().handle( e );
}
}