8000 Added num_steps for AsyncProcess as well as ability to GET/SET Var wh… by ysingh7 · Pull Request #591 · lava-nc/lava · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

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
Merged
merged 2 commits into from
Jan 31, 2023
Merged

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

merged 2 commits into from
Jan 31, 2023

Conversation

ysingh7
Copy link
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
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
Contributor

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
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
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

@joyeshmishra joyeshmishra joyeshmishra approved these changes

@bamsumit bamsumit bamsumit approved these changes

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
0