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

Synaptic delays for Dense connections #624

Merged
merged 37 commits into from
Feb 17, 2023
Merged

Conversation

PhilippPlank
Copy link
Contributor

@PhilippPlank PhilippPlank commented Feb 15, 2023

Issue Number: #237
Based on PR: #267

Objective of pull request: As a user, I want to be able to specify synaptic delays in the Dense Process to enable algorithmic work.

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?

What is the new behavior?

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

kds300 and others added 30 commits May 26, 2022 17:13
…he outputs, when it needs to be initialized to the size of the inputs
@PhilippPlank PhilippPlank self-assigned this Feb 15, 2023
@PhilippPlank PhilippPlank linked an issue Feb 15, 2023 that may be closed by this pull request
6 tasks
Copy link
Contributor

@bamsumit bamsumit left a comment

Choose a reason for hiding this comment

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

Thanks @PhilippPlank for taking care of this. It looks nice and clean. I have added some comments for your consideration.

tests/lava/proc/dense/test_models.py Show resolved Hide resolved
src/lava/proc/dense/models.py Outdated Show resolved Hide resolved
src/lava/proc/dense/process.py Show resolved Hide resolved
src/lava/proc/dense/process.py Show resolved Hide resolved
src/lava/proc/dense/process.py Show resolved Hide resolved
Copy link
Contributor

@gkarray gkarray left a comment

Choose a reason for hiding this comment

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

I left a single comment about potential performance improvement. Might even be not that critical if the overhead incurred there is not important.
Otherwise looks good! I'll approve to not block PR.

src/lava/proc/dense/models.py Outdated Show resolved Hide resolved
@bamsumit bamsumit merged commit e15b936 into lava-nc:main Feb 17, 2023
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* Added Delay connection process and process model (floating point).

* fixed bug in Delay() process. s_buff was initialized to the size of the outputs, when it needs to be initialized to the size of the inputs

* updated docstring for delay process

* added license information

* reformatting to improve line lengths

* updating formatting based on linting results

* added unit tests for delay process

* finished adding unittests and fixed linting errors

* added __init__.py file to the test directory for the delay process

* test_models.py import from tests.lava

* Added DenseDelay Process class, which inherits from Dense and adds delays to the connections.

* added PyDenseDelayModelFloat, the floating-point implementation for the DenseDelay process model.

* small fixes

* Added DelayDense process and process model

* fix linting

* deleted unused files

* deleted unused files

* modification from review suggestions

* modifications from reviewer suggestions

---------

Co-authored-by: kds300 <[email protected]>
Co-authored-by: kds300 <[email protected]>
Co-authored-by: Marcus G K Williams <[email protected]>
Co-authored-by: bamsumit <[email protected]>
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.

Synaptic delays in Dense Process
5 participants