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.

Sign up for GitHub

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

Jump to bottom

Partial implementation of virtual ports for PyProcModels #187

Merged
merged 21 commits into from
Feb 25, 2022
Merged

Partial implementation of virtual ports for PyProcModels #187

merged 21 commits into from
Feb 25, 2022

Conversation

mathisrichter
Copy link
Contributor

@mathisrichter mathisrichter commented Feb 17, 2022 •
edited
Loading

Issue Number: #185 (partial fulfilment)

Objective of pull request: Supplies an implementation of virtual ports (reshape, flatten, transpose) for PyProcModels

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 (pyb) passes locally
  • Build tests (pyb -E unit) or (python -m unittest) 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?

  • Virtual ports cannot be executed

What is the new behavior?

  • Virtual ports reshape, flatten, transpose can be executed with PyProcModels

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

bamsumit and others added 11 commits January 20, 2022 16:26
@bamsumit
permute initial implementation
d3a65e7 
Signed-off-by: bamsumit <[email protected]>
@bamsumit
Tests for permute ports
58d5a14
@mathisrichter
Process property of virtual ports no longer returns None
3891c8c 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
Added initial run-unittest for flatten() from issue lava-nc#163
eba5df9 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
User-level API for TransposePort with unit tests
917a497 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
Fixed typo
0f33774 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
Unit tests for flatten() and concat_with()
1537c41 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
Unit tests for virtual ports in Processes that are executed (wip)
938b7db 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
Preliminary implementation of virtual ports between OutPort and InPor…
d0eff5a 
…t (wip)

Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter
Merge branch 'main' of https://github.com/lava-nc/lava into permute
5076ab4 
� Conflicts:
�	src/lava/magma/compiler/compiler.py
�	src/lava/magma/core/model/py/ports.py
�	tests/lava/magma/core/model/py/test_ports.py
@mathisrichter
Fixing unit tests after merge
846c1b3 
Signed-off-by: Mathis Richter <[email protected]>
@mathisrichter mathisrichter added 1-feature New feature request help needed Extra attention is needed labels Feb 17, 2022
@mathisrichter mathisrichter self-assigned this Feb 17, 2022
PhilippPlank
PhilippPlank reviewed Feb 17, 2022
View reviewed changes
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.

I think this looks good so far. When the OutPorts are also added, we should pull some of the loops looking for virtual ports in the compiler into a sub method.

awintel
awintel requested changes Feb 18, 2022
View reviewed changes
Copy link
Contributor

@awintel awintel left a comment

Choose a reason for hiding this comment

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

Looks fine. I just have a few minor remarks.

I just think we should probably raise an error in one special case of branching/joining.

src/lava/magma/core/process/ports/ports.py Show resolved Hide resolved
src/lava/magma/core/process/ports/ports.py Outdated Show resolved Hide resolved
src/lava/magma/core/model/py/ports.py Outdated Show resolved Hide resolved
src/lava/magma/compiler/compiler.py Show resolved Hide resolved
@bamsumit
Copy link
Contributor

Looks good. Can confirm transpose, reshape and flatten work with hierarchical process.

awintel
awintel approved these changes Feb 22, 2022
View reviewed changes
Copy link
Contributor

@awintel awintel left a comment

Choose a reason for hiding this comment

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

Looks fine.

src/lava/magma/core/process/ports/ports.py Outdated Show resolved Hide resolved
bamsumit
bamsumit approved these changes Feb 23, 2022
View reviewed changes
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.

Looks good overall.

@mathisrichter mathisrichter marked this pull request as ready for review February 25, 2022 13:03
@mathisrichter
Copy link
Contributor Author

Merging this in its current form for the release. This PR only supports virtual ports for reshape, flatten, and transpose between an OutPort and an InPort. Hierarchical Processes as well as chains of virtual ports are supported as well. But joining and forking as well as connection to and from other types of Ports (RefPorts, VarPorts) are not yet supported.

@mathisrichter mathisrichter changed the title Virtual ports implementation for PyProcModels Partial implementation of virtual ports for PyProcModels Feb 25, 2022
@mathisrichter mathisrichter merged commit f8ec083 into lava-nc:main Feb 25, 2022
@mgkwill mgkwill linked an issue Mar 9, 2022 that may be closed by this pull request
Ability to permute PyPort #158
Closed
7 tasks
@mgkwill mgkwill removed a link to an issue Mar 9, 2022
Ability to permute PyPort #158
Closed
7 tasks
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
@mathisrichter @bamsumit @mgkwill
Partial implementation of virtual ports for PyProcModels (lava-nc#187)
5512bac 
* permute initial implementation

Signed-off-by: bamsumit <[email protected]>

* Tests for permute ports

* Process property of virtual ports no longer returns None

Signed-off-by: Mathis Richter <[email protected]>

* Added initial run-unittest for flatten() from issue lava-nc#163

Signed-off-by: Mathis Richter <[email protected]>

* User-level API for TransposePort with unit tests

Signed-off-by: Mathis Richter <[email protected]>

* Fixed typo

Signed-off-by: Mathis Richter <[email protected]>

* Unit tests for flatten() and concat_with()

Signed-off-by: Mathis Richter <[email protected]>

* Unit tests for virtual ports in Processes that are executed (wip)

Signed-off-by: Mathis Richter <[email protected]>

* Preliminary implementation of virtual ports between OutPort and InPort (wip)

Signed-off-by: Mathis Richter <[email protected]>

* Fixing unit tests after merge

Signed-off-by: Mathis Richter <[email protected]>

* Added support for virtual ports between an OutPort and InPort of two hierarchical Processes

Signed-off-by: Mathis Richter <[email protected]>

* Clean up, exceptions, and generic unit tests for virtual port topologies

Signed-off-by: Mathis Richter <[email protected]>

* Fixed linter issues

Signed-off-by: Mathis Richter <[email protected]>

* Raising an exception when executing ConcatPort

Signed-off-by: Mathis Richter <[email protected]>

Co-authored-by: bamsumit <[email protected]>
Co-authored-by: Marcus G K Williams <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhilippPlank PhilippPlank PhilippPlank left review comments

@bamsumit bamsumit bamsumit approved these changes

@awintel awintel awintel approved these changes

@mgkwill mgkwill Awaiting requested review from mgkwill

@GaboFGuerra GaboFGuerra Awaiting requested review from GaboFGuerra

Assignees

@mathisrichter mathisrichter

Labels
1-feature New feature request help needed Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mathisrichter @bamsumit @PhilippPlank @awintel @mgkwill