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

Custom connector models in NEST 2.12 #683

Closed
kappeld opened this Issue Mar 16, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@kappeld
Contributor

kappeld commented Mar 16, 2017

This issue effects the modified API for registering connector models at the new ModelManager of NEST 2.12.

We have developed a dynamically loadable Module for NEST that implements a reward-based learning rule in a custom connector model. To implement this synapse model it was necessary for us to create a new type of synapses that is guaranteed to be updated after a certain maximum time lag instead of only waiting for a presynaptic spike event. The best way to achieve this, we found, was to write a new connector model that takes care of the regular updates, and which is registered using NEST's standard register_connection_model, at the NEST kernel. This solution has proven quite efficient and flexible, and worked very well with NEST 2.10. We are sharing this Module already with two other groups and were planning to make it available to the public soon.

However, the feature that we were using to register our connector model has been removed in NEST 2.12, more precisely the method synindex register_connection_model_( ConnectorModel* ); of the ModelManager is now private. This makes it impossible for a user of NEST to develop custom connector models as far as I can see.

The feature we have been implementing (synapses that are updated on a regular basis) is something that is of greater interest, I believe, since it is needed for many interesting synapse models such as the reinforce / Clopath- like update rules. It would be a pity if this class of models is lost for the NEST community. Is there a plan to introduce this feature in NEST in one or the other way in future releases? To get this feature functioning again I would need one of the following things:

  1. Give the user again access to ModelManager->register_connection_model_( ConnectorModel* ); I do understand why you decided to make this method private since it somehow provides a "backdoor" which an inexperienced user could use to break many things. However, with this step you made it impossible for a user of NEST to write a new ConnectorModel. On the long run, giving developers who write custom models for NEST the opportunity to register synapses that use new connector models might be an important feature to increase the flexibility of NEST. On could do this, e.g. through a protected member function of ConnectorModel such that only developers that really want to write a new connector model have access to this.

  2. A second solution would be to put the option to have regular updates of synapse models into the NEST kernel. Since this feature might be interesting to many users of NEST, as described above, this option might be worthwhile considering. However, it probably comes with quite some overhead.

I'm quite agnostic to what solution is chosen to get this feature back (any of the two I proposed or another option). In any case I would be happy to provide code and help. For the first option I could easily implement a solution and make a pull request. Is there already a plan to realize something of this kind in a future version of NEST?

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Mar 16, 2017

Contributor

@kappeld ModelManager provides the public register_connection_model() method. Could you briefly explain why you cannot use this?

Contributor

heplesser commented Mar 16, 2017

@kappeld ModelManager provides the public register_connection_model() method. Could you briefly explain why you cannot use this?

@kappeld

This comment has been minimized.

Show comment
Hide comment
@kappeld

kappeld Mar 16, 2017

Contributor

Thank you for the quick answer! The new interface:

template < class ConnectionT > void ModelManager::register_connection_model( const std::string& name, bool requires_symmetric )

allows only to register the connection model with NEST's build in GenericConnectorModel (instantiated in line 91 of model_manager_impl.h). In our Module we had to write our own connector model which was derived from the build-in ConnectorModel to add some functionality (regular update of synapses). This was possible before, but is now impossible since the only public access is through the template function.

Contributor

kappeld commented Mar 16, 2017

Thank you for the quick answer! The new interface:

template < class ConnectionT > void ModelManager::register_connection_model( const std::string& name, bool requires_symmetric )

allows only to register the connection model with NEST's build in GenericConnectorModel (instantiated in line 91 of model_manager_impl.h). In our Module we had to write our own connector model which was derived from the build-in ConnectorModel to add some functionality (regular update of synapses). This was possible before, but is now impossible since the only public access is through the template function.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Mar 17, 2017

Contributor

@kappeld I understand your problem now. I think the most elegant way to solve this would be to add a second template parameter to the public register_connection_model() method with default value, along the lines of

template < typename ConnectionT, typename ConnectorModelT = GenericConnectorModel >
void
ModelManager::register_connection_model( ... )
{
  ConnectorModel* cf = new ConnectorModelT< ConnectionT >( ... );
  ...
  if ( ... )
 {
   cf = new ConnectorModelT< ConnectionLabel< ConnectionT > >( ... );
 }
}

The second variant is important for PyNN-support. Could you test this an create a pull request for it?

It also seems interesting to add your synapse models to NEST, since they provide support for a new class learning rules. Our main criteria for adding new features are that they should be scientifically relevant and that users who do not use them should not have to "pay" in form of slower simulation or bloated memory. Could you prepare a separate pull request for this new feature?

Contributor

heplesser commented Mar 17, 2017

@kappeld I understand your problem now. I think the most elegant way to solve this would be to add a second template parameter to the public register_connection_model() method with default value, along the lines of

template < typename ConnectionT, typename ConnectorModelT = GenericConnectorModel >
void
ModelManager::register_connection_model( ... )
{
  ConnectorModel* cf = new ConnectorModelT< ConnectionT >( ... );
  ...
  if ( ... )
 {
   cf = new ConnectorModelT< ConnectionLabel< ConnectionT > >( ... );
 }
}

The second variant is important for PyNN-support. Could you test this an create a pull request for it?

It also seems interesting to add your synapse models to NEST, since they provide support for a new class learning rules. Our main criteria for adding new features are that they should be scientifically relevant and that users who do not use them should not have to "pay" in form of slower simulation or bloated memory. Could you prepare a separate pull request for this new feature?

@kappeld

This comment has been minimized.

Show comment
Hide comment
@kappeld

kappeld Mar 17, 2017

Contributor

Thanks! that looks like a very elegant solution. I will try this out a.s.a.p and close the issue once I could verify that it works and then create the pull request.

Contributor

kappeld commented Mar 17, 2017

Thanks! that looks like a very elegant solution. I will try this out a.s.a.p and close the issue once I could verify that it works and then create the pull request.

kappeld pushed a commit to kappeld/nest-simulator that referenced this issue Mar 20, 2017

Kappel David Kappel David
Fix to issue #683.
- A second template argument to member function register_connection_model of the ModelManager has been added, which allows the user to specify the type of connector model.
- An overloaded function to register_connection_model has been added that uses the default connector model GenericConnectorModel.

kappeld pushed a commit to kappeld/nest-simulator that referenced this issue Mar 20, 2017

Kappel David Kappel David
Fix to issue #683.
- A second template argument to member function register_connection_model of the ModelManager has been added, which allows the user to specify the type of connector model.
- An overloaded function to register_connection_model has been added that uses the default connector model GenericConnectorModel.
@kappeld

This comment has been minimized.

Show comment
Hide comment
@kappeld

kappeld Mar 21, 2017

Contributor

I created a pull request that fixes this issue: #686

Sorry for multiple commits. I had some troubles with Vera++

Contributor

kappeld commented Mar 21, 2017

I created a pull request that fixes this issue: #686

Sorry for multiple commits. I had some troubles with Vera++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment