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

Ensure Connect() does not allow passing parameters that need to be set at the synapse-model level #3021

Merged
merged 8 commits into from
Nov 30, 2023
25 changes: 0 additions & 25 deletions models/jonke_synapse.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,6 @@ class jonke_synapse : public Connection< targetidentifierT >
*/
void set_status( const DictionaryDatum& d, ConnectorModel& cm );

/**
* Checks to see if illegal parameters are given in syn_spec.
*
* The illegal parameters are: "alpha", "beta", "lambda", "mu_plus", "mu_minus", "tau_plus", "Wmax"
*/
void check_synapse_params( const DictionaryDatum& d ) const;

/**
* Send an event to the receiver of this connection.
* \param e The event to send
Expand Down Expand Up @@ -390,24 +383,6 @@ jonke_synapse< targetidentifierT >::set_status( const DictionaryDatum& d, Connec
}
}

template < typename targetidentifierT >
void
jonke_synapse< targetidentifierT >::check_synapse_params( const DictionaryDatum& syn_spec ) const
{
std::string param_arr[] = { "alpha", "beta", "lambda", "mu_plus", "mu_minus", "tau_plus", "Wmax" };

const size_t n_param = sizeof( param_arr ) / sizeof( std::string );
for ( size_t n = 0; n < n_param; ++n )
{
if ( syn_spec->known( param_arr[ n ] ) )
{
std::string msg = "Connect doesn't support the setting of parameter " + param_arr[ n ]
+ " in jonke_synapse. Use SetDefaults() or CopyModel().";
throw NotImplemented( msg );
}
}
}

} // of namespace nest

#endif // of #ifndef JONKE_SYNAPSE_H
19 changes: 0 additions & 19 deletions models/stdp_dopamine_synapse.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,6 @@ template < typename targetidentifierT >
void
stdp_dopamine_synapse< targetidentifierT >::check_synapse_params( const DictionaryDatum& syn_spec ) const
{
if ( syn_spec->known( names::volume_transmitter ) )
{
throw NotImplemented(
"Connect doesn't support the direct specification of the "
"volume transmitter of stdp_dopamine_synapse in syn_spec."
"Use SetDefaults() or CopyModel()." );
}
// Setting of parameter c and n not thread safe.
if ( kernel().vp_manager.get_num_threads() > 1 )
{
Expand All @@ -418,18 +411,6 @@ stdp_dopamine_synapse< targetidentifierT >::check_synapse_params( const Dictiona
"Use SetDefaults() or CopyModel()." );
}
}
std::string param_arr[] = { "A_minus", "A_plus", "Wmax", "Wmin", "b", "tau_c", "tau_n", "tau_plus" };

const size_t n_param = sizeof( param_arr ) / sizeof( std::string );
for ( size_t n = 0; n < n_param; ++n )
{
if ( syn_spec->known( param_arr[ n ] ) )
{
std::string msg = "Connect doesn't support the setting of parameter " + param_arr[ n ]
+ " in stdp_dopamine_synapse. Use SetDefaults() or CopyModel().";
throw NotImplemented( msg );
}
}
}

template < typename targetidentifierT >
Expand Down
5 changes: 2 additions & 3 deletions models/tsodyks_synapse_hom.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,9 @@ class tsodyks_synapse_hom : public Connection< targetidentifierT >
void
set_weight( double )
{
throw BadProperty(
throw NotImplemented(
heplesser marked this conversation as resolved.
Show resolved Hide resolved
"Setting of individual weights is not possible! The common weights can "
"be changed via "
"CopyModel()." );
"be changed via CopyModel()." );
}

private:
Expand Down
2 changes: 2 additions & 0 deletions nestkernel/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ class Connection
*
* @note Classes requiring checks need to override the function with their own
* implementation, as this base class implementation does not do anything.
*
* @see ConnectorModel::check_synapse_params
*/
void check_synapse_params( const DictionaryDatum& d ) const;

Expand Down
12 changes: 6 additions & 6 deletions nestkernel/connector_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ class ConnectorModel

/**
* Checks to see if illegal parameters are given in syn_spec.
*
* Checks against setting CommonSynapseProperties upon Connect() are implemented in GenericConnectorModel.
* Any further checks need to be implemented by the connection model class by overriding
* Connection::check_synapse_params().
*/
virtual void check_synapse_params( const DictionaryDatum& ) const = 0;

Expand Down Expand Up @@ -182,12 +186,6 @@ class GenericConnectorModel : public ConnectorModel
void get_status( DictionaryDatum& ) const override;
void set_status( const DictionaryDatum& ) override;

void
check_synapse_params( const DictionaryDatum& syn_spec ) const override
{
default_connection_.check_synapse_params( syn_spec );
}

typename ConnectionT::CommonPropertiesType const&
get_common_properties() const override
{
Expand All @@ -196,6 +194,8 @@ class GenericConnectorModel : public ConnectorModel

void set_syn_id( synindex syn_id ) override;

void check_synapse_params( const DictionaryDatum& syn_spec ) const override;

SecondaryEvent*
get_secondary_event() override
{
Expand Down
22 changes: 22 additions & 0 deletions nestkernel/connector_model_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,28 @@ GenericConnectorModel< ConnectionT >::set_status( const DictionaryDatum& d )
default_delay_needs_check_ = true;
}

template < typename ConnectionT >
void
GenericConnectorModel< ConnectionT >::check_synapse_params( const DictionaryDatum& syn_spec ) const
{
// This is called just once per Connect() call, so we need not worry much about performance.
// We get a dictionary with synapse default values and check if any of its keys are in syn_spec.
DictionaryDatum dummy( new Dictionary );
cp_.get_status( dummy );

for ( [[maybe_unused]] const auto& [ key, val ] : *syn_spec )
{
if ( dummy->known( key ) )
{
throw NotImplemented(
heplesser marked this conversation as resolved.
Show resolved Hide resolved
String::compose( "Synapse parameter \"%1\" can only be set via SetDefaults() or CopyModel().", key ) );
}
}

default_connection_.check_synapse_params( syn_spec );
}


template < typename ConnectionT >
void
GenericConnectorModel< ConnectionT >::used_default_delay()
Expand Down
135 changes: 135 additions & 0 deletions testsuite/pytests/sli2py_connect/test_common_properties_setting.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# -*- coding: utf-8 -*-
#
# test_common_properties_setting.py
#
# 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/>.

"""
Test that common properties can be Set/Read via Defaults, but not individual connections.

These tests only check the setting/getting mechanics for one parameter per synapse to confirm
that the overall mechanics "work". Testing the setting of specific parameters must be done in
model-specific tests.
"""

import nest
import pytest


def set_volume_transmitter():
vt = nest.Create("volume_transmitter")
nest.SetDefaults("stdp_dopamine_synapse", {"volume_transmitter": vt})


def set_default_delay_resolution():
nest.resolution = nest.GetDefaults("eprop_synapse")["delay"]


# This list shall contain all synapse models extending the CommonSynapseProperties class.
# For each model, specify which parameter to test with and which test value to use. A
# setup function can be provided if preparations are required. Provide also supported neuron model.
common_prop_models = {
"eprop_synapse": {
"parameter": "batch_size",
"value": 10,
"setup": set_default_delay_resolution,
"neuron": "eprop_iaf_psc_delta",
},
"jonke_synapse": {"parameter": "tau_plus", "value": 10, "setup": None, "neuron": "iaf_psc_alpha"},
"stdp_dopamine_synapse": {
"parameter": "tau_plus",
"value": 10,
"setup": set_volume_transmitter,
"neuron": "iaf_psc_alpha",
},
"stdp_facetshw_synapse_hom": {"parameter": "tau_plus", "value": 10, "setup": None, "neuron": "iaf_psc_alpha"},
"stdp_pl_synapse_hom": {"parameter": "tau_plus", "value": 10, "setup": None, "neuron": "iaf_psc_alpha"},
"stdp_synapse_hom": {"parameter": "tau_plus", "value": 10, "setup": None, "neuron": "iaf_psc_alpha"},
"static_synapse_hom_w": {"parameter": "weight", "value": 10, "setup": None, "neuron": "iaf_psc_alpha"},
"tsodyks_synapse_hom": {"parameter": "tau_psc", "value": 10, "setup": None, "neuron": "iaf_psc_alpha"},
}

# Filter models that may not be built in
available_cp_models = {k: v for k, v in common_prop_models.items() if k in nest.synapse_models}


@pytest.fixture(autouse=True)
def reset_kernel():
nest.ResetKernel()


@pytest.mark.parametrize("syn_model, specs", available_cp_models.items())
def test_set_common_properties(syn_model, specs):
"""Test that setting a parameter works"""

if specs["setup"]:
specs["setup"]()

old_val = nest.GetDefaults(syn_model)[specs["parameter"]]
assert old_val != specs["value"] # test would be meaningless otherwise

nest.SetDefaults(syn_model, {specs["parameter"]: specs["value"]})
new_val = nest.GetDefaults(syn_model)[specs["parameter"]]
assert new_val == specs["value"]


@pytest.mark.parametrize("syn_model, specs", available_cp_models.items())
def test_copy_common_properties(syn_model, specs):
"""Test that parameters set on a model are copied and that setting on the copy does not touch the original"""

if specs["setup"]:
specs["setup"]()

old_val = nest.GetDefaults(syn_model)[specs["parameter"]]
assert old_val != specs["value"] # test would be meaningless otherwise

nest.SetDefaults(syn_model, {specs["parameter"]: specs["value"]})
new_model = syn_model + "_copy"
nest.CopyModel(syn_model, new_model)
new_val = nest.GetDefaults(new_model)[specs["parameter"]]
assert new_val == specs["value"]

# Set parameter back on copied model, original must not be changed
nest.SetDefaults(new_model, {specs["parameter"]: old_val})
assert nest.GetDefaults(syn_model)[specs["parameter"]] == specs["value"]


@pytest.mark.parametrize("syn_model, specs", available_cp_models.items())
def test_no_setting_on_connection(syn_model, specs):
"""Test that common property cannot be set on individual connection"""

if specs["setup"]:
specs["setup"]()

n = nest.Create(specs["neuron"])
nest.Connect(n, n, syn_spec={"synapse_model": syn_model})
conn = nest.GetConnections()
with pytest.raises(nest.kernel.NESTErrors.DictError):
conn.set({specs["parameter"]: specs["value"]})


@pytest.mark.parametrize("syn_model, specs", available_cp_models.items())
def test_no_setting_on_connect(syn_model, specs):
"""Test that common property cannot be set in a Connect call"""

if specs["setup"]:
specs["setup"]()

n = nest.Create(specs["neuron"])
with pytest.raises(nest.kernel.NESTErrors.NotImplemented):
nest.Connect(n, n, syn_spec={"synapse_model": syn_model, specs["parameter"]: specs["value"]})
91 changes: 0 additions & 91 deletions testsuite/unittests/test_common_props_setting.sli

This file was deleted.

Loading