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

LIF refractory floating point #655

Merged
merged 7 commits into from
Jun 2, 2023

Conversation

ssgier
Copy link
Contributor

@ssgier ssgier commented Mar 21, 2023

Issue Number: #282

Objective of pull request: Add LIF process with refractory period

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?

  • No LIF process with refractory period is available

What is the new behavior?

  • LIF process with refractory period is available. CPU/Floating point process model that implements the process is available.

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

As discussed with @mathisrichter, I created a new process based on the LIF process with reset. This change currently only includes the floating point process model in Python as a start. It could serve as a starting point to discuss/implement other process models.

@mathisrichter
Copy link
Contributor

Oh boy, sorry @ssgier for the long delay. I have not been monitoring Github for a while and we overlooked this PR.
I have now assigned a few people from my team to review it.

Hopefully, we can review and get this merged soon now.
My apologies!

Copy link
Contributor

@weidel-p weidel-p left a comment

Choose a reason for hiding this comment

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

Thanks for this clean implementation there is not much to ask for.

The only thing is, that I would like to change the default value to 0 instead of 1. In your current implementation, 1 means "no refractoryness" and yields the same results as the normal LIF. In my understanding, 1 should mean that the voltage is clamped to zero for one timestep after spiking. Hence I would change the line

non_refractory = self.refractory_period_end <= self.time_step

to

non_refractory = self.refractory_period_end < self.time_step

and allow refractory_period to be 0 but not negative.

src/lava/proc/lif/process.py 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.

Looks good. Some minor comments from my side ;)

tests/lava/proc/lif/test_models.py Outdated Show resolved Hide resolved
tests/lava/proc/lif/test_models.py Outdated Show resolved Hide resolved
@PhilippPlank PhilippPlank merged commit b8e014a into lava-nc:main Jun 2, 2023
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* LIF refractory float

* Fix off-by-one bug

* Comment style consistency

---------

Co-authored-by: PhilippPlank <[email protected]>
Co-authored-by: Mathis Richter <[email protected]>
Co-authored-by: weidel-p <[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.

4 participants