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

Dummy axons work properly on chip #261

Merged
merged 6 commits into from Jan 22, 2020
Merged

Dummy axons work properly on chip #261

merged 6 commits into from Jan 22, 2020

Conversation

hunse
Copy link
Collaborator

@hunse hunse commented Nov 15, 2019

We were having problems with dummy axons still doing something on the chip, even though they shouldn't. This PR fixes that. I added the test in a separate commit first, so that it's easy to confirm the test fails before the fix and passes after it.

The latter two commits improve support for population axons. The first one allows users to make a connection use pop16 axons instead of the default pop32. This uses fewer axon slots, so that connections can have more axons, but adds some constraints to how these axons are used. (In convolutional networks, this functionally amounts to having to use numbers of filters that are a multiple of 4, with channels_last=True. This allows the axon compartment offset ("base") to be a multiple of 4, which is the requirement for pop16. We could make copies of the weights in the case that we need offsets that are not multiples of 4, such that we have up to 4 copies each for a different modulo of the offset. See #262.)

The last commit allows population axons to be used on input connections when precompute=True (since NxSDK's spike generators now support them).

Based on #260.

@hunse hunse force-pushed the dummy-axon-host-snips branch 5 times, most recently from c6a8d04 to 26b8400 Compare Nov 26, 2019
@drasmuss drasmuss changed the base branch from master to host-snips Nov 29, 2019
@hunse hunse mentioned this pull request Dec 4, 2019
1 task
@tbekolay tbekolay force-pushed the host-snips branch 6 times, most recently from 1aeca78 to 9255d68 Compare Dec 20, 2019
@hunse hunse mentioned this pull request Jan 14, 2020
@arvoelke
Copy link
Collaborator

@arvoelke arvoelke commented Jan 14, 2020

Leaving reminder here: after #260 should be able to change the splitter logic to allow any convolution connections for both precompute True and False.

@tbekolay tbekolay changed the base branch from host-snips to master Jan 20, 2020
Copy link
Member

@tbekolay tbekolay left a comment

Made a few minor changes to code in fixups, and some relatively large changes to the changelog entries. In general, we want the changelog entries to be short (2-3 lines tops). If more explanation is needed, it should be in a more permanent location. This is because we expect only existing users to be looking through the changelog, so if an important detail is hidden there, new users trying to learn about Nengo Loihi will miss it. Even for existing users, we want the changelog to be something short and sweet that they can read in its entirety in a few minutes. If it gets too long then they just won't read it at all.

Will merge on @hunse and @kinjalpatel27's 👍 of the new fixups.

nengo_loihi/hardware/nxsdk_objects.py Outdated Show resolved Hide resolved
nengo_loihi/hardware/nxsdk_objects.py Outdated Show resolved Hide resolved
nengo_loihi/config.py Outdated Show resolved Hide resolved
@tbekolay tbekolay force-pushed the dummy-axon-host-snips branch 3 times, most recently from 9bc7365 to 6d354e8 Compare Jan 22, 2020
@tbekolay tbekolay mentioned this pull request Jan 22, 2020
For population connections, these dummy axons were still having
an effect. The synapses associated with these axons cause index
errors in the NxSDK axon compiler, so those are removed as well.

This commit also clarifies axon indices versus IDs.

Fixes the "invalid index" error raised in #246.
kinjalpatel27 and others added 5 commits Jan 22, 2020
This allows population spikes to be sent from host to chip
when `precompute=True`.

Also add spikes to spike generator more quickly by doing
deobfuscation once only per group of spikes.

Co-authored-by: Eric Hunsberger <eric.hunsberger@appliedbrainresearch.com>
We were accidentally returning the Loihi weights, due to double usage
of the term `weights`. This is now less ambiguous.
We avoid the restriction of the axon compartment bases being less
than 256, by taking any part of the required offset over this
and adding it into the indices instead.

We also clarify the `conv2d_loihi_weights` function and
remove nengo-dl from `test_conv.py:test_conv_connection`.
The nengo-dl portions have not been run in a long time, since they
were only enabled if `channels_last=True`, and we were x-failing
the test for `channels_last=True` due to lack of nengo-loihi
support. It contributes nothing to testing nengo-loihi, so we remove it.
Copy link
Member

@tbekolay tbekolay left a comment

I squashed the history a bit after merging #246 into this PR. It all looks ready to go to me now, so will merge on @hunse's OK.

@tbekolay tbekolay merged commit 7d1aab1 into master Jan 22, 2020
3 checks passed
@tbekolay tbekolay deleted the dummy-axon-host-snips branch Jan 22, 2020
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

4 participants