Skip to content

Commit

Permalink
Merge pull request #177 from heplesser/fix105-bad-min-delay
Browse files Browse the repository at this point in the history 
Fix #105: improve and fix setting of min_ and max_delay
  • Loading branch information
@jougs
jougs committed Dec 21, 2015
2 parents 77fba86 + 1449e2f commit 336e89a
Show file tree
Hide file tree
Showing 4 changed files with 289 additions and 97 deletions.
89 changes: 46 additions & 43 deletions nestkernel/connector_model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,72 +58,65 @@ ConnectorModel::ConnectorModel( const ConnectorModel& cm, const std::string name
max_delay_.calibrate();
}

void
ConnectorModel::update_delay_extrema( const double_t mindelay_cand, const double_t maxdelay_cand )
{
// check min delay candidate
Time delay_cand = Time( Time::ms( mindelay_cand ) );
if ( delay_cand < min_delay_ )
min_delay_ = delay_cand;

// check max delay candidate
delay_cand = Time( Time::ms( maxdelay_cand ) );
if ( delay_cand > max_delay_ )
max_delay_ = delay_cand;
}

void
ConnectorModel::assert_valid_delay_ms( double_t requested_new_delay )
{
// We have to convert the delay in ms to a Time object then to steps and back the ms again
// in order to get the value in ms which can be represented with an integer number of steps
// in the currently chosen Time representation.
// See also bug #217, MH 08-04-23
// This is also done by creating a Time object out of the provided ms.
const Time new_delay = Time( Time::ms( requested_new_delay ) );
const delay new_delay = Time::delay_ms_to_steps( requested_new_delay );
const double new_delay_ms = Time::delay_steps_to_ms( new_delay );

if ( new_delay < Time::get_resolution() )
throw BadDelay( new_delay.get_ms(), "Delay must be greater than or equal to resolution" );
if ( new_delay < Time::get_resolution().get_steps() )
throw BadDelay( new_delay_ms, "Delay must be greater than or equal to resolution" );

// if already simulated, the new delay has to be checked against the
// min_delay and the max_delay which have been used during simulation
if ( net_.get_simulated() )
{
Time sim_min_delay = Time::step( net_.get_min_delay() );
Time sim_max_delay = Time::step( net_.get_max_delay() );
const bool bad_min_delay = new_delay < sim_min_delay;
const bool bad_max_delay = new_delay > sim_max_delay;
const bool bad_min_delay = new_delay < net_.get_min_delay();
const bool bad_max_delay = new_delay > net_.get_max_delay();

if ( bad_min_delay || bad_max_delay )
throw BadDelay( new_delay.get_ms(),
"Minimum and maximum delay cannot be changed after Simulate has been called." );
throw BadDelay( new_delay_ms,
"Minimum and maximum delay cannot be changed "
"after Simulate has been called." );
}

const bool new_min_delay = new_delay < min_delay_;
const bool new_max_delay = new_delay > max_delay_;
const bool new_min_delay = new_delay < min_delay_.get_steps();
const bool new_max_delay = new_delay > max_delay_.get_steps();

if ( new_min_delay )
{
if ( user_set_delay_extrema_ )
throw BadDelay( new_delay.get_ms(), "Delay must be greater than or equal to min_delay." );
{
throw BadDelay( new_delay_ms,
"Delay must be greater than or equal to min_delay. "
"You may set min_delay before creating connections." );
}
else
update_delay_extrema( new_delay.get_ms(), max_delay_.get_ms() );
{
min_delay_ = Time( Time::step( new_delay ) );
}
}

if ( new_max_delay )
{
if ( user_set_delay_extrema_ )
throw BadDelay( new_delay.get_ms(), "Delay must be smaller than or equal to max_delay." );
{
throw BadDelay( new_delay_ms,
"Delay must be smaller than or equal to max_delay. "
"You may set min_delay before creating connections." );
}
else
update_delay_extrema( min_delay_.get_ms(), new_delay.get_ms() );
{
max_delay_ = Time( Time::step( new_delay ) );
}
}
}

void
ConnectorModel::assert_two_valid_delays_steps( long_t new_delay1, long_t new_delay2 )
ConnectorModel::assert_two_valid_delays_steps( delay new_delay1, delay new_delay2 )
{
const long_t ldelay = std::min( new_delay1, new_delay2 );
const long_t hdelay = std::max( new_delay1, new_delay2 );
const delay ldelay = std::min( new_delay1, new_delay2 );
const delay hdelay = std::max( new_delay1, new_delay2 );

if ( ldelay < Time::get_resolution().get_steps() )
throw BadDelay(
Expand All @@ -149,19 +142,29 @@ ConnectorModel::assert_two_valid_delays_steps( long_t new_delay1, long_t new_del
if ( new_min_delay )
{
if ( user_set_delay_extrema_ )
throw BadDelay(
Time::delay_steps_to_ms( ldelay ), "Delay must be greater than or equal to min_delay." );
{
throw BadDelay( Time::delay_steps_to_ms( ldelay ),
"Delay must be greater than or equal to min_delay. "
"You may set min_delay before creating connections." );
}
else
update_delay_extrema( Time::delay_steps_to_ms( ldelay ), max_delay_.get_ms() );
{
min_delay_ = Time( Time::step( ldelay ) );
}
}

if ( new_max_delay )
{
if ( user_set_delay_extrema_ )
throw BadDelay(
Time::delay_steps_to_ms( hdelay ), "Delay must be smaller than or equal to max_delay." );
{
throw BadDelay( Time::delay_steps_to_ms( hdelay ),
"Delay must be smaller than or equal to max_delay. "
"You may set max_delay before creating connections." );
}
else
update_delay_extrema( min_delay_.get_ms(), Time::delay_steps_to_ms( hdelay ) );
{
max_delay_ = Time( Time::step( hdelay ) );
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions nestkernel/connector_model.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,13 @@ class ConnectorModel
{
return min_delay_;
}

const Time&
get_max_delay() const
{
return max_delay_;
}

void update_delay_extrema( const double_t mindelay_cand, const double_t maxdelay_cand );

/**
* NAN is a special value in cmath, which describes double values that
* are not a number. If delay or weight is omitted in an add_connection call,
Expand All @@ -117,6 +116,7 @@ class ConnectorModel
synindex syn_id,
double_t delay = numerics::nan,
double_t weight = numerics::nan ) = 0;

virtual ConnectorBase* add_connection( Node& src,
Node& tgt,
ConnectorBase* conn,
Expand Down Expand Up @@ -168,7 +168,7 @@ class ConnectorModel
* working with continuous delays.
* @note Not const, since it may update delay extrema as a side-effect.
*/
void assert_two_valid_delays_steps( long_t, long_t );
void assert_two_valid_delays_steps( delay, delay );

std::string
get_name() const
Expand Down
97 changes: 46 additions & 51 deletions nestkernel/connector_model_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,76 +155,71 @@ GenericConnectorModel< ConnectionT >::set_status( const DictionaryDatum& d )
updateValue< long_t >( d, names::music_channel, receptor_type_ );
#endif

/*
* In the following code, we do not round delays to steps. For min and max delay,
* this is not strictly necessary. For a newly set delay, the rounding will be
* handled in cp_.set_status() or default_connection_.set_status().
* Since min_/max_delay are Time-objects and comparison is defined on Time
* objects, we should use it.
*/
Time min_delay, max_delay, new_delay;
// For the minimum delay, we always round down. The easiest way to do this,
// is to round up and then subtract one step. The only remaining edge case
// is that the min delay is exactly at a step, in which case one would get
// a min delay that is one step too small. We can detect this by an
// additional test.
double_t delay_tmp;
bool min_delay_updated = updateValue< double_t >( d, "min_delay", delay_tmp );
min_delay = Time( Time::ms( delay_tmp ) );
bool max_delay_updated = updateValue< double_t >( d, "max_delay", delay_tmp );
max_delay = Time( Time::ms( delay_tmp ) );
Time new_min_delay;
if ( min_delay_updated )
{
delay new_min_delay_steps = Time( Time::ms_stamp( delay_tmp ) ).get_steps();
if ( Time( Time::step( new_min_delay_steps ) ).get_ms() > delay_tmp )
{
new_min_delay_steps -= 1;
}
new_min_delay = Time( Time::step( new_min_delay_steps ) );
}

// the delay might also be updated, so check new_min_delay and new_max_delay against new_delay, if
// given
if ( !updateValue< double_t >( d, "delay", delay_tmp ) )
new_delay = Time( Time::ms( default_connection_.get_delay() ) );
else
new_delay = Time( Time::ms( delay_tmp ) );
// For the maximum delay, we always round up, using ms_stamp
bool max_delay_updated = updateValue< double_t >( d, "max_delay", delay_tmp );
Time new_max_delay = Time( Time::ms_stamp( delay_tmp ) );

if ( min_delay_updated xor max_delay_updated )
net_.message(
SLIInterpreter::M_ERROR, "SetDefaults", "Both min_delay and max_delay have to be specified" );
{
throw BadProperty( "Both min_delay and max_delay have to be specified" );
}

if ( min_delay_updated && max_delay_updated )
{
if ( num_connections_ > 0 )
net_.message( SLIInterpreter::M_ERROR,
"SetDefaults",
"Connections already exist. Please call ResetKernel first" );
else if ( min_delay > new_delay )
net_.message(
SLIInterpreter::M_ERROR, "SetDefaults", "min_delay is not compatible with default delay" );
else if ( max_delay < new_delay )
net_.message(
SLIInterpreter::M_ERROR, "SetDefaults", "max_delay is not compatible with default delay" );
else if ( min_delay < Time::get_resolution() )
net_.message( SLIInterpreter::M_ERROR,
"SetDefaults",
"min_delay must be greater than or equal to resolution" );
else if ( max_delay < Time::get_resolution() )
net_.message( SLIInterpreter::M_ERROR,
"SetDefaults",
"max_delay must be greater than or equal to resolution" );
{
throw BadProperty( "Connections already exist. Please call ResetKernel first" );
}
else if ( new_min_delay < Time::get_resolution() )
{
throw BadDelay(
new_min_delay.get_ms(), "min_delay must be greater than or equal to resolution." );
}
else if ( new_max_delay < new_min_delay )
{
throw BadDelay(
new_min_delay.get_ms(), "min_delay must be smaller than or equal to max_delay." );
}
else
{
min_delay_ = min_delay;
max_delay_ = max_delay;
min_delay_ = new_min_delay;
max_delay_ = new_max_delay;
user_set_delay_extrema_ = true;
}
}

// common_props_.set_status(d, *this) AND defaults_.set_status(d, *this);
// has to be done after adapting min_delay / max_delay, since Connection::set_status
// and CommonProperties::set_status might want to check the delay
// If the parameter dict d contains /delay, this should set the delay
// on the default connection, but not affect the actual min/max_delay
// until a connection with that default delay is created. Since the
// set_status calls on common properties and default connection may
// modify min/max delay, we need to preserve and restore.

// store min_delay_, max_delay_
// calling set_status will check the delay.
// and so may modify min_delay, max_delay, if the specified delay exceeds one of these bounds
// we have to save min/max_delay because we dont know, if the default will ever be used
Time min_delay_tmp = min_delay_;
Time max_delay_tmp = max_delay_;
const Time save_min_delay_ = min_delay_;
const Time save_max_delay_ = max_delay_;

cp_.set_status( d, *this );
default_connection_.set_status( d, *this );

// restore min_delay_, max_delay_
min_delay_ = min_delay_tmp;
max_delay_ = max_delay_tmp;
min_delay_ = save_min_delay_;
max_delay_ = save_max_delay_;

// we've possibly just got a new default delay. So enforce checking next time it is used
default_delay_needs_check_ = true;
Expand All @@ -236,7 +231,7 @@ GenericConnectorModel< ConnectionT >::used_default_delay()
{
// if not used before, check now. Solves bug #138, MH 08-01-08
// replaces whole delay checking for the default delay, see bug #217, MH 08-04-24
// get_default_delay_ must be overridded by derived class to return the correct default delay
// get_default_delay_ must be overridden by derived class to return the correct default delay
// (either from commonprops or default connection)
if ( default_delay_needs_check_ )
{
Expand Down
Loading

0 comments on commit 336e89a

Please sign in to comment.