Skip to content

Added num_steps for AsyncProcess as well as ability to GET/SET Var wh…#591

Merged
joyeshmishra merged 2 commits intomainfrom
asyncp
Jan 31, 2023
Merged

Added num_steps for AsyncProcess as well as ability to GET/SET Var wh…#591
joyeshmishra merged 2 commits intomainfrom
asyncp

Conversation

@ysingh7
Copy link
Copy Markdown
Contributor

@ysingh7 ysingh7 commented Jan 30, 2023

…en its not in RUNNING State.

Issue Number:

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?

What is the new behavior?

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

@awintel
Copy link
Copy Markdown
Contributor

awintel commented Jan 30, 2023

What's the intention here?
num_steps + AsyncProc looks a bit like a hack as an AsyncProc does not have a notion of time.
If this is necessary short term, have you filed another story to resolve the root cause requiring this hack?

@bamsumit
Copy link
Copy Markdown
Contributor

bamsumit commented Jan 31, 2023

What's the intention here? num_steps + AsyncProc looks a bit like a hack as an AsyncProc does not have a notion of time. If this is necessary short term, have you filed another story to resolve the root cause requiring this hack?

@awintel The intention of the num_steps is to communicate how many timesteps the model is intended to run. I do understand that it is not absolutely necessary and the Async Model does not need to use it. This additional information of num_steps

  • provides a way to stop the async model execution after the intended timestep.
  • makes get/set vars at the end of the execution possible.
  • allows us to dynamically convert models using Loihi protocol to Async which will speed up the lava inference.

@awintel
Copy link
Copy Markdown
Contributor

awintel commented Jan 31, 2023

Ok thanks.
I'm not against doing this in the short term. But we should revise this architectural awkwardness.

@ysingh7
Copy link
Copy Markdown
Contributor Author

ysingh7 commented Jan 31, 2023

Ok thanks. I'm not against doing this in the short term. But we should revise this architectural awkwardness.

@awintel I have created an issue to track this : #592.

@ysingh7 ysingh7 closed this Jan 31, 2023
@ysingh7 ysingh7 reopened this Jan 31, 2023
@joyeshmishra joyeshmishra merged commit aa84ac2 into main Jan 31, 2023
@joyeshmishra joyeshmishra deleted the asyncp branch January 31, 2023 04:33
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
@ysingh7
Added num_steps for AsyncProcess as well as ability to GET/SET Var wh… (
ff511ca 
lava-nc#591)

* Added num_steps for AsyncProcess as well as ability to GET/SET Var when its not in RUNNING State.

* Fixed lint issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bamsumit bamsumit bamsumit approved these changes

+1 more reviewer

@joyeshmishra joyeshmishra joyeshmishra approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ysingh7 @awintel @bamsumit @joyeshmishra