Skip to content

Fix bug with no inputs to a neuron#246

Merged
tbekolay merged 2 commits into
dummy-axon-host-snipsfrom
fix-sparse-transforms
Jan 20, 2020
Merged

Fix bug with no inputs to a neuron#246
tbekolay merged 2 commits into
dummy-axon-host-snipsfrom
fix-sparse-transforms

Conversation

@hunse

@hunse hunse commented Aug 14, 2019

Copy link
Copy Markdown
Contributor

Some arrays have shape (1, 0) which gives them a length of 1, but no size.

@tbekolay tbekolay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix looked good, but wasn't tested. I added in the short example from #245 and also added a changelog entry. Will merge when CI finishes successfully.

@tbekolay tbekolay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Turns out that this change works fine on emulator, but fails on hardware with the following warning from NxSDK:

Connection error: invalid index

For some reason, rather than failing to build, NxSDK goes into some kind of loop where it will keep trying (unsuccessfully) to build the network.

I'm not really sure where to start debugging that, so I'm going to send the PR back to you @hunse.

Comment thread nengo_loihi/tests/test_connection.py Outdated


@pytest.mark.skipif(nengo_transforms is None, reason="Requires new nengo.transforms")
def test_sparse_transforms(Simulator):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe test_sparse_transform_empty_neuron or something like that, since we're specifically testing sparse transforms that have some neurons that get no inputs, not sparse transforms generally. Also, we should put this description in a little docstring.

@hunse

hunse commented Nov 22, 2019

Copy link
Copy Markdown
Contributor Author

Sounds good. Thanks for the test!

hunse added a commit that referenced this pull request Dec 13, 2019
These are synapses with `base == None` or no weights. Building them
not only uses extra resources, but can cause index errors in the
NxSDK axon compiler.

Fixes the "invalid index" error raised in #246.
@hunse hunse force-pushed the fix-sparse-transforms branch from b89f607 to 3ad3a3b Compare December 13, 2019 19:35
@hunse

hunse commented Dec 13, 2019

Copy link
Copy Markdown
Contributor Author

Ok, I think I've fixed the issue. 🤞 The solution was to not build synapses that have no weights, which is something that I meant to do at some point anyway. I had to rebase onto #261, so this will have to wait a bit until that's merged first.

@hunse

hunse commented Dec 13, 2019

Copy link
Copy Markdown
Contributor Author

Also, I've got this as two commits right now, but I'd be fine squashing them (maybe to something like "Fix sparse connections with no inputs to some neurons").

hunse added 2 commits January 20, 2020 14:43
These are synapses with `base == None` or no weights. Building them
not only uses extra resources, but can cause index errors in the
NxSDK axon compiler.

Fixes the "invalid index" error raised in #246.
@tbekolay tbekolay force-pushed the fix-sparse-transforms branch from 41ef09f to 779d445 Compare January 20, 2020 19:44
@tbekolay tbekolay changed the base branch from master to dummy-axon-host-snips January 20, 2020 19:44
@tbekolay tbekolay merged commit 779d445 into dummy-axon-host-snips Jan 20, 2020
@tbekolay tbekolay deleted the fix-sparse-transforms branch January 20, 2020 20:08
tbekolay pushed a commit that referenced this pull request Jan 22, 2020
These are synapses with `base == None` or no weights. Building them
not only uses extra resources, but can cause index errors in the
NxSDK axon compiler.

Fixes the "invalid index" error raised in #246.
tbekolay pushed a commit that referenced 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.

Fixes the "invalid index" error raised in #246.
tbekolay pushed a commit that referenced 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.
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.

2 participants