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

Fix for dangling ports #274

Merged
merged 18 commits into from
Jul 22, 2022
Merged

Fix for dangling ports #274

merged 18 commits into from
Jul 22, 2022

Conversation

bamsumit
Copy link
Contributor

Issue Number: 246

Objective of pull request:

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?

  • Dangling input/output port does not work.

What is the new behavior?

  • They work

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

@bamsumit bamsumit self-assigned this Jul 21, 2022
@bamsumit bamsumit linked an issue Jul 21, 2022 that may be closed by this pull request
8 tasks
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.

Looks good, Sumit! It may be worth adding a dedicated unit test for this though to make sure this capability does not break in the future.

@bamsumit
Copy link
Contributor Author

Looks good, Sumit! It may be worth adding a dedicated unit test for this though to make sure this capability does not break in the future.

Yeah I was looking at the best place to add that. I am not convinced it should go in test_process.py though. Suggestions?

Signed-off-by: bamsumit <[email protected]>
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.

Nice! I think the unit test is fine there. Since it is a feature of the compiler, you could also have written it as a part of the Compiler tests and only tested the contents of the builders, for instance. But this is also nice, then we have a working example. :)

tests/lava/magma/runtime/test_io_ports.py Show resolved Hide resolved
tests/lava/magma/runtime/test_io_ports.py Outdated Show resolved Hide resolved
Signed-off-by: bamsumit <[email protected]>
@bamsumit bamsumit merged commit c9c74b2 into lava-nc:main Jul 22, 2022
@bamsumit bamsumit deleted the danglin_port_fix branch July 22, 2022 16:12
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* update refport unittest to always wait when it writes to port for consistent behavior

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

* Removed pyproject changes

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

* Fix to convolution tests. Fixed imcompatible mnist_pretrained for old python versions.

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

* Dangling port fix update

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

* Fix dangling port without breaking channel map unittests

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

* Remove unnecessary hack for dangling port in tutorial06

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

* Added dangling port unittests

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

* Linting fix

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

* Typo fixes

Signed-off-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.

Support Dangling Ports during Compilation and Execution
4 participants