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

Add function to activate direct mode. #1168

Merged
merged 1 commit into from
Feb 10, 2017
Merged

Add function to activate direct mode. #1168

merged 1 commit into from
Feb 10, 2017

Conversation

jgosmann
Copy link
Collaborator

@jgosmann jgosmann commented Sep 8, 2016

Motivation and context:
Sometime I want to run networks in direct mode, but some parts use connections to or from neurons. In those cases it is not possible to set the top-level neuron_type to Direct because some ensembles need to still use neurons. This PR adds a function where direct mode is only activated for ensembles where this is possible.

Not sure whether this better fits into nengo_extras, but to me it seems like something pretty basic that one might need for many networks.

How has this been tested?
Added a unit test.

How long should this take to review?

  • Average (neither quick nor lengthy)

Types of changes:

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@mention-bot
Copy link

@jgosmann, thanks for your PR! By analyzing the annotation information on this pull request, we identified @tbekolay to be a potential reviewer

@tbekolay
Copy link
Member

tbekolay commented Sep 8, 2016

it seems like something pretty basic that one might need for many networks

+1 for having it in Nengo! Handy thing to have around for sure.

for c in network.all_connections:
if isinstance(c.pre, nengo.ensemble.Neurons):
requires_neurons.add(c.pre.ensemble)
if isinstance(c.post, nengo.ensemble.Neurons):
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be c.pre_obj and c.post_obj so that you can catch situations like nengo.Connection(a.neurons[0], b.neurons[0])

@tcstewar
Copy link
Contributor

tcstewar commented Sep 8, 2016

Nice... yup, there's been a few times where I've wanted this sort of thing. I've even tried a few times to implement something like this that will also add in a bit of saturation behaviour (by modifying the function on the Connections), but never got it to a state I was happy about.....

@hunse
Copy link
Collaborator

hunse commented Sep 9, 2016

LGTM too.

My inclination was to say this belongs in nengo_extras, but it's okay here, too.

CHANGES.rst Outdated
@@ -53,6 +53,10 @@ Release History
`#1148 <https://github.com/nengo/nengo/pull/1148>`_)
- Tweaked ``rasterplot`` so that spikes from different neurons don't overlap.
(`#1121 <https://github.com/nengo/nengo/pull/1121>`_)
- Added ``activate_direcd_mode`` function to make it easier to activate direct
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo.

@jgosmann jgosmann self-assigned this Nov 11, 2016
@jgosmann
Copy link
Collaborator Author

Since I wrote this, it occured to me that this function should also handle learning rules which might require non-direct mode ensembles.

@jgosmann jgosmann removed their assignment Feb 8, 2017
@jgosmann
Copy link
Collaborator Author

jgosmann commented Feb 8, 2017

Excluded learning rule ensembles from direct mode. This is now ready for a re-review.

Copy link
Contributor

@Seanny123 Seanny123 left a comment

Choose a reason for hiding this comment

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

LGTM. I added a quick commit to fix the typo in CHANGES.rst.

This function respects ensembles where neurons are required because
of connections to/from neurons, or if learning rules are present,
none of which are the case when setting the neuron_type in the
top-level config.

Closes #1111.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants