Skip to content

Bugfix to enable monitoring aliased vars#675

Merged
PhilippPlank merged 7 commits intolava-nc:mainfrom
AlessandroPierro:bugfix-monitor-aliased-var
May 2, 2023
Merged

Bugfix to enable monitoring aliased vars#675
PhilippPlank merged 7 commits intolava-nc:mainfrom
AlessandroPierro:bugfix-monitor-aliased-var

Conversation

@AlessandroPierro
Copy link
Copy Markdown

@AlessandroPierro AlessandroPierro commented Apr 27, 2023

Issue Number: #676

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

What is the current behavior?

  • An assertion was raised when it was requested to create two different implicit var ports with the same name on the same process

What is the new behavior?

  • The different var ports will be created without assertion, with a numerical suffix added to the name

Does this introduce a breaking change?

  • No

@AlessandroPierro
Copy link
Copy Markdown
Author

Required to merge: lava-nc/lava-optimization#217

@PhilippPlank PhilippPlank self-assigned this Apr 27, 2023
@PhilippPlank PhilippPlank added the 1-bug Something isn't working label Apr 27, 2023
@PhilippPlank PhilippPlank linked an issue Apr 27, 2023 that may be closed by this pull request
Copy link
Copy Markdown
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.

This PR adds a suffix to a port name if it already exists which is an easy solution to the problem described in the issue.

I have one request though, could you please add a test which shows that the original issue is solved? So that we can probe aliased vars?

Comment thread src/lava/magma/core/process/ports/ports.py Outdated
@PhilippPlank PhilippPlank merged commit ae45028 into lava-nc:main May 2, 2023
@AlessandroPierro AlessandroPierro deleted the bugfix-monitor-aliased-var branch May 2, 2023 10:01
@tim-shea tim-shea added this to the Lava v0.7.1 milestone May 15, 2023
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* Bugfix to enable monitoring aliased vars

* Update ports.py

* Update ports.py

* Update test_ports.py

* Update test_ports.py

* Address review feedback

* Match expected behaviour for implicit var port naming

---------

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

Labels

1-bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to probe aliased vars with Monitor

4 participants