Update runtime and process to be "with" context managers.

[Gavinator98]

Issue Number: #599

Objective of pull request: Update runtime and processes to be Python "with" context managers so it is easier to automatically call stop().

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?

• Runtime and processes do not support "with" statements.

What is the new behavior?

• Runtime and processes both support "with" statements.
• When entering a with block the runtime is initialized
• When exiting a with block the runtime is stopped.

Example:

with process_to_run:
    process_to_run.run(condition=RunSteps(num_steps=1000), run_cfg=run_config)

Does this introduce a breaking change?

• Yes
• No

Supplemental information

[weidel-p]

That's a clean and easy implementation and include tests.

If you want, you could demonstrate this feature in the tutorial about execution:
https://github.com/lava-nc/lava/blob/main/tutorials/in_depth/tutorial04_execution.ipynb

[mathisrichter]

Hi @Gavinator98

This looks very nice. Thanks for your work on this feature!

You are testing this feature with the RunContinuous run condition only, which uses a non-blocking call by default. Could you please also add tests for the RunSteps condition? One with RunSteps(num_steps=..., blocking=True) (the default) and one with RunSteps(num_steps=..., blocking=False). It should be fine to only test these variants on the Process. You do not have to also test them for the runtime.

I also added a few minor suggestions. Could you please address those as well?

Thanks,
Mathis

[Gavinator98]

Hi @Gavinator98

This looks very nice. Thanks for your work on this feature!

You are testing this feature with the RunContinuous run condition only, which uses a non-blocking call by default. Could you please also add tests for the RunSteps condition? One with RunSteps(num_steps=..., blocking=True) (the default) and one with RunSteps(num_steps=..., blocking=False). It should be fine to only test these variants on the Process. You do not have to also test them for the runtime.

I also added a few minor suggestions. Could you please address those as well?

Thanks, Mathis

Thanks for the feedback, I'll try and address your comments early next week. One question, most of the test code is derived from /tests/lava/magma/runtime/test_run_continuously_and_pause.py Would it make sense to refactor SimpleProcess, etc for both into a utils module?

[mathisrichter]

Sure, removing code duplication of unit tests is a valuable goal. Go for it and refactor shared code into a module but make sure you do not introduce dependencies between unit tests, where the behavior of one test is influenced by another.
It may also be worth renaming the Processes and ProcessModels. I feel that SimpleProcess is not very descriptive and could just be named Process or something short that explains what it is for.

You will notice that we have a lot of code duplication in unit test infrastructure throughout Lava tests... Maybe you will set a good example here that others will follow. Thanks for your initiative!

[Gavinator98]

On further review, a majority of the tests have similar classes to SimpleProcess with slightly different names, variables, and number of processes. I don't think it makes sense to make that change anymore since it would only be shared by two of the test classes.

[mathisrichter]

Thanks, @Gavinator98 for this great new addition to Lava! If you are interested in contributing further in some form, do not hesitate to reach out. :)