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

Alternative to Injector/Extractor Processes #835

Merged
merged 5 commits into from
Mar 19, 2024

Conversation

gkarray
Copy link
Contributor

@gkarray gkarray commented Feb 8, 2024

Issue Number:

Objective of pull request: Give an alternative (better) implementation of Injector and Extractor main functionality without wrapping it into a Lava Process.

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): Alternative implementation of a feature

What is the current behavior?

  • Injector and Extractor Processes are used to communicate data in and out of Lava.

What is the new behavior?

  • Communication between Lava and the outside would be done differently. The API would look like this:
relay = Relay(shape=(1,)) # Process with an InPort (in_1) and an OutPort (out_1)
...
relay.run(condition=RunStep(num_steps=1, blocking=False), ...)
...
send_data = np.array([5])
relay.in_1.external_pipe_csv_send_port.send(send_data)
...
recv_data = relay.out_1.external_pipe_csv_recv_port.recv()
  • Advantages of this alternative:
    • Uses the same MultiProcessing instance as the Runtime (addressing pending problem highlighted in IO bridge Processes #686).
    • Communication to/from the target Lava ports is direct, while with the Injector/Extractor, it had to go through an intermediary channel.

Does this introduce a breaking change?

  • Yes
  • No
  • If we delete the Injector and Extractor Processes in this PR, any code using them has to be adapted to use the alternative instead.

@PhilippPlank PhilippPlank added the 1-refactor Suggestion to refactor part of the code label Feb 9, 2024
@PhilippPlank PhilippPlank marked this pull request as ready for review March 4, 2024 09:40
@bamsumit
Copy link
Contributor

bamsumit commented Mar 5, 2024

This looks nice guys. Does it mean that we will get rid of injector and extractor processes as it is now? I would not delete them right now and maybe provide deprecation warning that it will be removed in lava vX.Y.Z. This will allow all the subsequent apps to make the transition.

@PhilippPlank
Copy link
Contributor

This looks nice guys. Does it mean that we will get rid of injector and extractor processes as it is now? I would not delete them right now and maybe provide deprecation warning that it will be removed in lava vX.Y.Z. This will allow all the subsequent apps to make the transition.

We have rewritten the Injector/Extractor classes, so they are fully backwards compatible with the new change. They do offer some functionality which might be handy, e.g., what happens to the data when the buffer is full (drop, stall, etc.). So one could continue using this processes in the future, but it is not needed anymore. I am open to deprecate them in the future, but can also see value in keeping them.

@bamsumit
Copy link
Contributor

This looks nice guys. Does it mean that we will get rid of injector and extractor processes as it is now? I would not delete them right now and maybe provide deprecation warning that it will be removed in lava vX.Y.Z. This will allow all the subsequent apps to make the transition.

We have rewritten the Injector/Extractor classes, so they are fully backwards compatible with the new change. They do offer some functionality which might be handy, e.g., what happens to the data when the buffer is full (drop, stall, etc.). So one could continue using this processes in the future, but it is not needed anymore. I am open to deprecate them in the future, but can also see value in keeping them.

Great. In that case we do not need to deprecate injector and extractor.

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.

I have nothing to add to this discussion. The code looks fine to me, great work!

@PhilippPlank PhilippPlank merged commit 67559c2 into main Mar 19, 2024
6 checks passed
@PhilippPlank PhilippPlank deleted the injector_extractor_rework branch March 19, 2024 09:14
srrisbud added a commit that referenced this pull request May 16, 2024
* Explicitly keep track of all Processes in the Executable to make sure all Processes are assigned the Runtime

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* A utility to measure the size of a python object recursively. Useful for example, to measure the size of NxBoard object after compilation

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* Removed the utility to estimate Python object size

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* Fixed Runtime unittest to accommodate passing an explicit ProcessList to the Executable

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* Removed unused import

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* LearningDense bit-accurate (#812)

* minor change in dependency computation

* updating stochastic round type hint

* small fix in clip_weights

* progress in making tests pass

* fixing Sparse init

* trying tests

* adapting init method of LearningDense Process

---------

Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>

* Bump fonttools from 4.41.1 to 4.43.0 (#824)

Bumps [fonttools](https://github.com/fonttools/fonttools) from 4.41.1 to 4.43.0.
- [Release notes](https://github.com/fonttools/fonttools/releases)
- [Changelog](https://github.com/fonttools/fonttools/blob/main/NEWS.rst)
- [Commits](fonttools/fonttools@4.41.1...4.43.0)

---
updated-dependencies:
- dependency-name: fonttools
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump gitpython from 3.1.37 to 3.1.41 (#825)

Bumps [gitpython](https://github.com/gitpython-developers/GitPython) from 3.1.37 to 3.1.41.
- [Release notes](https://github.com/gitpython-developers/GitPython/releases)
- [Changelog](https://github.com/gitpython-developers/GitPython/blob/main/CHANGES)
- [Commits](gitpython-developers/GitPython@3.1.37...3.1.41)

---
updated-dependencies:
- dependency-name: gitpython
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* SigmaS4Delta Neuronmodel and Layer with Unittests (#830)

* first wokring version

* S4D model cleaned

* update license

* fix imports

* linting

* incorporate reviews

* update docstring

* Bump pillow from 10.0.1 to 10.2.0 (#832)

Bumps [pillow](https://github.com/python-pillow/Pillow) from 10.0.1 to 10.2.0.
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@10.0.1...10.2.0)

---
updated-dependencies:
- dependency-name: pillow
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>

* [QUBO] Solution readout via spikeIO for multi-chip support (#820)

* 32bit receiver

* linting

---------

Co-authored-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* Bump cryptography from 41.0.6 to 42.0.0 (#834)

Bumps [cryptography](https://github.com/pyca/cryptography) from 41.0.6 to 42.0.0.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@41.0.6...42.0.0)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump cryptography from 42.0.0 to 42.0.2 (#836)

Bumps [cryptography](https://github.com/pyca/cryptography) from 42.0.0 to 42.0.2.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@42.0.0...42.0.2)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump cryptography from 42.0.2 to 42.0.4 (#837)

Bumps [cryptography](https://github.com/pyca/cryptography) from 42.0.2 to 42.0.4.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@42.0.2...42.0.4)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Spiker with 32bit

* Alternative to Injector/Extractor Processes (#835)

* prototype implementing injector/extractor function, not wrapped in a Process

* modified injector and extractor classes

* fixed linting

---------

Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>
Co-authored-by: Philipp Plank <philipp.plank@intel.com>

* fix security issues

* Fix workflows (#844)

* prototype implementing injector/extractor function, not wrapped in a Process

* modified injector and extractor classes

* fixed linting

* fix workflows

---------

Co-authored-by: gkarray <ghassen.karray@intel.com>

* Bump pillow from 10.2.0 to 10.3.0 (#847)

Bumps [pillow](https://github.com/python-pillow/Pillow) from 10.2.0 to 10.3.0.
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@10.2.0...10.3.0)

---
updated-dependencies:
- dependency-name: pillow
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump idna from 3.6 to 3.7 (#848)

Bumps [idna](https://github.com/kjd/idna) from 3.6 to 3.7.
- [Release notes](https://github.com/kjd/idna/releases)
- [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst)
- [Commits](kjd/idna@v3.6...v3.7)

---
updated-dependencies:
- dependency-name: idna
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update ci.yml (#853)

* ATRLIF neuron model (#846)

* added process and CPU process models of ATRLIF neuron; added a Jupyter notebook to demonstrate the properties of the ATRLIF neuron

* testing of the tutorial

* tests for process and model added; copyright notes added; cleanup

* codacy-related fixed

* comment formatting and copyright notices adjusted

* Add the models and process of conv_in_time in src/lava/proc/conv_in_time (#833)

* add the models and process of conv_in_time in src/lava/proc/conv_in_time

* remove unused library

* remove Trailing whitespace

* add unittest for conv in time and related pytorch ground truth

* add fixed_pt version of conv in time

* change input to spike_input

* add from lava.proc.conv import utils

* remove unwanted comments

* fixed some linting errors

* Start all comments with upper case character & change the year for all copyright headers to 2024 for new files

* remove whitespace

* continuation line under-indented

* Trailing whitespace

* shorten variables names

---------

Co-authored-by: bamsumit <bam_sumit@hotmail.com>
Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>

* Fixed/updated poetry.lock to match the TOML file

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* Delinting

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

* Fixed a unittest by adding .name attribute to a Mock object

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>

---------

Signed-off-by: Risbud, Sumedh <sumedh.risbud@intel.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Ghassen Karray <36999864+gkarray@users.noreply.github.com>
Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: smm-ncl <145007783+smm-ncl@users.noreply.github.com>
Co-authored-by: Philipp Stratmann <86950058+phstratmann@users.noreply.github.com>
Co-authored-by: Philipp Plank <philipp.plank@intel.com>
Co-authored-by: gkarray <ghassen.karray@intel.com>
Co-authored-by: Jannik Luboeinski <33398515+jlubo@users.noreply.github.com>
Co-authored-by: Zeyu Liu <liuzeyu@usc.edu>
Co-authored-by: bamsumit <bam_sumit@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-refactor Suggestion to refactor part of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants