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

CIFAR-10 on Loihi #161

Merged
merged 5 commits into from Feb 8, 2019
Merged

CIFAR-10 on Loihi #161

merged 5 commits into from Feb 8, 2019

Conversation

@hunse
Copy link
Collaborator

@hunse hunse commented Jan 4, 2019

This has CIFAR-10 running on Loihi. It replaces #117.

The main feature addition here is allowing population spikes on inputs. This allows us to re-use axons in the three-channel CIFAR-10 input images, making it much easier to get things on the chip.

A lot of these commits can probably be squashed together. The script in the sandbox should also be moved to the sandbox repository.

@hunse
Copy link
Collaborator Author

@hunse hunse commented Jan 18, 2019

  • TODO: remove xfails in test_conv tests that were waiting for the ability to send pop spikes to chip.
  • move sandbox script to separate repo

@hunse hunse force-pushed the cifar-notebook branch from 44e966b to 7b35f33 Jan 30, 2019
@hunse
Copy link
Collaborator Author

@hunse hunse commented Jan 30, 2019

So when I re-enable the test_conv tests, test_conv.test_conv_connection does not work for two-channel inputs. I need to look more into why this is. The rest of this could be reviewed (i.e. the notebook is not going to change much).

@hunse hunse force-pushed the cifar-notebook branch 2 times, most recently from 55e9410 to 65f7b27 Jan 31, 2019
@hunse
Copy link
Collaborator Author

@hunse hunse commented Jan 31, 2019

Ok, this should be ready to go. The failing test was just because we weren't upping the snip spikes per step (maybe that default should be higher? Also, we've got quite a few mostly useless warnings now that get spewed for most tests, so we might want to clean those up to make real warnings like this one more obvious).

It's ready for review, and I've rebased it so hopefully everything passes. As for squashing it, the first three commits can be squashed no problem. The "Added ImageShifter" commit could go with the notebook commit. The last commit could probably go with the first ones, since it's just enabling tests that relate to that functionality.

@hunse
Copy link
Collaborator Author

@hunse hunse commented Jan 31, 2019

The "docs" test is failing because it can't run the new notebook, because it needs the GPU TensorFlow. The CPU TensorFlow doesn't support having the channels first in conv, which is what I use here. We might be able to write our TensorFlow code to get around this, basically by explicitly transposing things if we realize we're on the CPU. Thoughts?

@tbekolay tbekolay force-pushed the cifar-notebook branch 3 times, most recently from e22cfc6 to b81f32b Feb 4, 2019
Copy link
Member

@tbekolay tbekolay left a comment

Added some fixups and separate commits for the C file. Not sure if the commits to the C file should be separate or squashed into one 🤷‍♂️ But with those changes, this looks good to me.

@tbekolay
Copy link
Member

@tbekolay tbekolay commented Feb 4, 2019

Note: merging this is blocked by nengo/nengo-examples#19 as we link to the example added in that PR here.

@hunse
Copy link
Collaborator Author

@hunse hunse commented Feb 4, 2019

All @tbekolay's additions/changes look good to me. Just two minor comments.

hunse and others added 5 commits Feb 8, 2019
This commit also makes spike packing more efficient,
and removes the 'xfail' mark from tests that require
population spikes.
Instead of having to comment / uncomment print statements.
This value should not change, but it makes the code that uses
it more readable.
No camel case for our own code (unavoidable for NxSDK things),
and one variable definition per line.
@tbekolay tbekolay force-pushed the cifar-notebook branch from a2426fa to d0bfed1 Feb 8, 2019
@tbekolay tbekolay merged commit d0bfed1 into master Feb 8, 2019
2 of 3 checks passed
@tbekolay tbekolay deleted the cifar-notebook branch Feb 8, 2019
@tbekolay tbekolay mentioned this pull request Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants