Skip to content

RefPort's sometimes handled a time step late#205

Merged
PhilippPlank merged 42 commits intolava-nc:mainfrom
PhilippPlank:refport_wait
Mar 2, 2022
Merged

RefPort's sometimes handled a time step late#205
PhilippPlank merged 42 commits intolava-nc:mainfrom
PhilippPlank:refport_wait

Conversation

@PhilippPlank
Copy link
Copy Markdown
Contributor

@PhilippPlank PhilippPlank commented Mar 1, 2022

Issue Number: #204

Objective of pull request: Deterministic behavior of when data sent via a RefPort is received by the target process. Occasionally it was possible that the receiving process moved to the next phase before all of its VarPorts got serviced, because sending data via a RefPort does not block. Introducing of a wait() method for RefPorts makes sure that sent data is serviced before the process finishes (which blocks advancing a time step and ensures all VarPorts on the receiving process will be serviced beforehand).

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?

  • Occasionally it was possible that the receiving process moved to the next phase before all of its VarPorts got serviced, because sending data via a RefPort does not block.

What is the new behavior?

  • Introducing of a wait() method for RefPorts makes sure that sent data is serviced before the process finishes (which blocks advancing a time step and ensures all VarPorts on the receiving process will be serviced beforehand).

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

PhilippPlank and others added 30 commits November 12, 2021 14:53
@PhilippPlank PhilippPlank self-assigned this Mar 1, 2022
@PhilippPlank PhilippPlank requested review from awintel and bamsumit March 1, 2022 21:26
@PhilippPlank PhilippPlank linked an issue Mar 1, 2022 that may be closed by this pull request
7 tasks
@PhilippPlank PhilippPlank added 1-bug Something isn't working area: magma/core Issues with something in lava/magma/core labels Mar 1, 2022
Copy link
Copy Markdown
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. But you should update the docstring.

Comment thread src/lava/proc/io/reset.py Outdated
Comment thread src/lava/magma/core/model/py/ports.py Outdated
Comment thread src/lava/proc/io/reset.py Outdated
@bamsumit
Copy link
Copy Markdown
Contributor

bamsumit commented Mar 2, 2022

Tested it out on PN SNN notebook. The the results are consistent.

- added wait() to the dataloader
- modified docstring of wait
@PhilippPlank PhilippPlank requested a review from awintel March 2, 2022 09:11
Copy link
Copy Markdown
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.

I would still recommend to improve the docstring.
The fundamental behavior of the RefPort should be explained in the class docstring.

Approving anyways since this is just a style issue that can be fixed later. Just don't forget about it.

Comment thread src/lava/magma/core/model/py/ports.py Outdated
@PhilippPlank PhilippPlank merged commit b746bb7 into lava-nc:main Mar 2, 2022
@PhilippPlank PhilippPlank deleted the refport_wait branch March 2, 2022 17:20
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* - Introducing of a wait() method for RefPorts makes sure that sent data is serviced before the process finishes (which blocks advancing a time step and ensures all VarPorts on the receiving process will be serviced beforehand).

* - optimized Reset by not using a read() to get the data shape
- added wait() to the dataloader
- modified docstring of wait

* - modified docstring of wait
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 area: magma/core Issues with something in lava/magma/core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RefPort's sometimes handled a time step late

3 participants