Skip to content

Fix get set var#256

Merged
PhilippPlank merged 47 commits intolava-nc:mainfrom
PhilippPlank:fix_get_set_var
Jun 14, 2022
Merged

Fix get set var#256
PhilippPlank merged 47 commits intolava-nc:mainfrom
PhilippPlank:fix_get_set_var

Conversation

@PhilippPlank
Copy link
Copy Markdown
Contributor

@PhilippPlank PhilippPlank commented Jun 13, 2022 •
edited
Loading

Issue Number: #255

Objective of pull request: Get/Set Var works as expected when running in continuous mode

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?

  • Get/Set Var fails running in continuous mode

What is the new behavior?

  • Get/Set Var gives the values of respective Vars when running in continuous mode with paused execution

Does this introduce a breaking change?

  • Yes
  • No

PhilippPlank and others added 30 commits November 12, 2021 14:53
@PhilippPlank
- Initial enablement of RefPort and VarPorts
f456afb
@PhilippPlank
- Initial enablement of RefPort and VarPorts
2d0ec74
@PhilippPlank
- Initial enablement of RefPort and VarPorts
25a3a68
@PhilippPlank
- Initial enablement of RefPort and VarPorts
a2d1765
@PhilippPlank
Merge branch 'lava-nc:main' into main
74dfdf6
@PhilippPlank
- Enablement of RefPorts and VarPorts - addressed change requests fro…
1daa9af 
…m PR lava-nc#46
@PhilippPlank
- Enablement of RefPorts and VarPorts - addressed change requests fro…
5908401 
…m PR lava-nc#46
@PhilippPlank
- Enablement of RefPorts and VarPorts - addressed change requests fro…
3df2847 
…m PR lava-nc#46
@PhilippPlank
Merge branch 'lava-nc:main' into main
9581fae
@PhilippPlank
- Enablement of RefPorts and VarPorts - addressed change requests fro…
6e4716a 
…m PR lava-nc#46
@PhilippPlank
Merge branch 'lava-nc:main' into main
e22def6
@PhilippPlank
Merge branch 'lava-nc:main' into main
bd25a80
@PhilippPlank
Merge branch 'lava-nc:main' into main
7edeb1f
@PhilippPlank
Merge branch 'lava-nc:main' into main
23fb8d7
@PhilippPlank
Merge branch 'lava-nc:main' into main
f6686b4
@PhilippPlank
Merge branch 'lava-nc:main' into main
86867c2
@PhilippPlank
Merge branch 'lava-nc:main' into main
859a195
@PhilippPlank
Merge branch 'lava-nc:main' into main
f7007f8
@PhilippPlank
Merge branch 'lava-nc:main' into main
0163202
@PhilippPlank
modified connection tutorial for release 0.2.0
bf934ef
@PhilippPlank
Merge branch 'lava-nc:main' into main
0eef67e
@PhilippPlank
fixed typos
f08a35c
@PhilippPlank
Merge branch 'lava-nc:main' into main
ceddf2a
@PhilippPlank
Merge branch 'lava-nc:main' into main
f798b0a
@PhilippPlank
Merge branch 'lava-nc:main' into main
b6e72bb
@PhilippPlank
Merge branch 'lava-nc:main' into main
9c57309
@PhilippPlank
Merge branch 'lava-nc:main' into main
7425f7e
@PhilippPlank
Merge branch 'lava-nc:main' into main
bc353c7
@PhilippPlank
Merge branch 'lava-nc:main' into main
73288a4
@PhilippPlank
Merge branch 'lava-nc:main' into main
3f5c2d7
@PhilippPlank PhilippPlank added 1-bug Something isn't working area: magma/runtime Issues with something in lava/magma/runtime labels Jun 13, 2022
@PhilippPlank PhilippPlank self-assigned this Jun 13, 2022
GaboFGuerra
GaboFGuerra approved these changes Jun 13, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@GaboFGuerra GaboFGuerra 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!

Comment thread src/lava/magma/runtime/runtime_services/runtime_service.py Outdated
phstratmann
phstratmann approved these changes Jun 13, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@phstratmann phstratmann 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 to me, except for the minor comment by GG.

Thanks for adding a unit test for this!

mathisrichter
mathisrichter approved these changes Jun 14, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Great!

Comment thread src/lava/magma/runtime/runtime_services/runtime_service.py Outdated
Comment thread src/lava/magma/runtime/runtime_services/runtime_service.py Outdated
@PhilippPlank PhilippPlank merged commit 430f8ec into lava-nc:main Jun 14, 2022
@PhilippPlank PhilippPlank deleted the fix_get_set_var branch June 14, 2022 13:58
@PhilippPlank PhilippPlank restored the fix_get_set_var branch June 14, 2022 14:03
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
@PhilippPlank
Fix get set var (lava-nc#256)
917f6c1 
* Fixing get/set Var when running a process continuously + Unit test

* Cleaned up pause/stop requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mathisrichter mathisrichter mathisrichter approved these changes

@GaboFGuerra GaboFGuerra GaboFGuerra approved these changes

@weidel-p weidel-p Awaiting requested review from weidel-p

+1 more reviewer

@phstratmann phstratmann phstratmann approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@PhilippPlank PhilippPlank

Labels

1-bug Something isn't working area: magma/runtime Issues with something in lava/magma/runtime

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PhilippPlank @mathisrichter @GaboFGuerra @phstratmann