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

Graded spike payload in pre-synaptic trace updates - floating-pt and bit-approximate-loihi #607

Merged
merged 47 commits into from
Feb 21, 2023

Conversation

gkarray
Copy link
Contributor

@gkarray gkarray commented Feb 6, 2023

Issue Number: #606

Objective of pull request: As a user of the learning API, I would like to use graded spike payloads to update pre-synaptic traces in order to implement more complex learning algorithms. Especially, this is critical for prototype-based learning approaches.

Pull request checklist

Your PR fulfills the following requirements:

  • Issue created that explains the change and why it's needed
  • Tests are part of the PR (for bug fixes / features)
  • Docs reviewed and added / updated if needed (for bug fixes / features)
  • PR conforms to Coding Conventions
  • PR applys BSD 3-clause or LGPL2.1+ Licenses to all code files
  • Lint (flakeheaven lint src/lava tests/) and (bandit -r src/lava/.) pass locally
  • Build tests (pytest) passes locally

Pull request type

Please check your PR type:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation changes
  • Other (please describe):

What is the current behavior?

  • Currently, feature described by the user story is not present.

What is the new behavior?

  • The feature described by the user story is present.

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

Copy link
Contributor

@mathisrichter mathisrichter left a comment

Choose a reason for hiding this comment

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

I have not reviewed this to a point, where I could approve or block it, so I am just leaving comments.

It all looks good though, at least from a high level coding-style point of view. I only added a few minor remarks.

The one thing we could maybe improve are the known-value tests. It is not clear where the known values come from or how we know they are correct. There are also so many that the significance of individual numbers is unclear. Please keep in mind that someone will at some point look at this code and need to understand why all these values as deemed correct. Make sure that it is very easy to understand that from the code. That person could be yourself one year from now. :)

src/lava/magma/core/model/py/connection.py Outdated Show resolved Hide resolved
src/lava/magma/core/model/py/connection.py Outdated Show resolved Hide resolved
src/lava/magma/core/learning/constants.py Outdated Show resolved Hide resolved
src/lava/magma/core/learning/constants.py Outdated Show resolved Hide resolved
src/lava/magma/core/model/py/connection.py Outdated Show resolved Hide resolved
src/lava/magma/core/model/py/connection.py Outdated Show resolved Hide resolved
src/lava/magma/core/model/py/connection.py Show resolved Hide resolved
tests/lava/proc/dense/test_learning.py Outdated Show resolved Hide resolved
Copy link
Contributor

@PhilippPlank PhilippPlank left a comment

Choose a reason for hiding this comment

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

This concept of how the payload modifies the traces seems quite complicated. We should have a good example in the learning tutorial to show what will happen.
If there is no issue for that yet, please create one.

@gkarray gkarray merged commit f209c56 into main Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Graded spike payload in pre-synaptic trace updates - floating-pt and bit-approximate-loihi
3 participants