Skip to content

Implement Sparse transforms#240

Merged
tbekolay merged 2 commits intomasterfrom
sparse-transforms
Aug 6, 2019
Merged

Implement Sparse transforms#240
tbekolay merged 2 commits intomasterfrom
sparse-transforms

Conversation

@hunse
Copy link
Copy Markdown
Contributor

@hunse hunse commented Jul 26, 2019

Support Sparse transforms for on-chip connections.

Also uses sparse matrices when possible for node->neuron and neuron->neuron connections that have a scalar or diagonal transform.

@hunse hunse force-pushed the sparse-transforms branch from 3a2ff27 to 5e72278 Compare July 29, 2019 12:52
@drasmuss drasmuss force-pushed the sparse-transforms branch 2 times, most recently from ee00573 to 54c7b16 Compare August 1, 2019 16:17
drasmuss
drasmuss reviewed Aug 1, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@drasmuss drasmuss left a comment

Choose a reason for hiding this comment

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

Added some minor fixups, and then two discussion comments for larger changes.

@drasmuss
Copy link
Copy Markdown
Member

drasmuss commented Aug 1, 2019

Oh, I also rebased this onto #241

@drasmuss drasmuss force-pushed the sparse-transforms branch from edd0eab to 16784b3 Compare August 2, 2019 15:53
drasmuss
drasmuss approved these changes Aug 5, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@drasmuss drasmuss left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Added some more fixups to simplify the sparse logic by adding scipy as a required dependency, get diff coverage up to 100%, and fixed a minor issue with unused imports I noticed while doing that. With that last batch of fixups, this looks good to me!

tbekolay
tbekolay approved these changes Aug 6, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@tbekolay tbekolay left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This LGTM, merging.

Oh, right the one thing this is missing is a changelog entry for the scipy requirement. I'll add that as I clean up the history.

hunse and others added 2 commits August 6, 2019 12:06
@hunse @drasmuss @tbekolay
Support Sparse transforms
e6a5600 
Also use sparse matrices when possible for node->neuron and
neuron->neuron connections.

Note that scipy is now a requirement for nengo-loihi.

Co-authored-by: Daniel Rasmussen <daniel.rasmussen@appliedbrainresearch.com>
@drasmuss @tbekolay
Remove unused imports
e15e02b 
We don't need __future__ anymore since we're Python 3 only.

Enable the pylint unused-imports check now that these are fixed.
@tbekolay tbekolay force-pushed the sparse-transforms branch from f84224b to e15e02b Compare August 6, 2019 16:06
@tbekolay tbekolay merged commit e15e02b into master Aug 6, 2019
@tbekolay tbekolay deleted the sparse-transforms branch August 6, 2019 17:07
@arvoelke
Copy link
Copy Markdown
Contributor

arvoelke commented Aug 14, 2019 •
edited
Loading

Should the documentation be sync'd up to reflect scipy dependency now? Edit: Created an issue (#244).

@arvoelke arvoelke mentioned this pull request Aug 14, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbekolay tbekolay tbekolay approved these changes

@drasmuss drasmuss drasmuss approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hunse @drasmuss @arvoelke @tbekolay