Skip to content
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.

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

Remove hacks and refactor Runtime and RuntimeService/Synchronizer #86

Closed
awintel opened this issue Nov 21, 2021 · 2 comments
Closed

Remove hacks and refactor Runtime and RuntimeService/Synchronizer #86

awintel opened this issue Nov 21, 2021 · 2 comments
Assignees
Labels
1-bug Something isn't working area: magma/runtime Issues with something in lava/magma/runtime

Comments

@awintel
Copy link
Contributor

awintel commented Nov 21, 2021

The Runtime and RuntimeService do currently contain a number of hacks that need to be cleaned up:

  • The Runtime has a 'current_ts' attribute. But the Runtime must not be aware of algorithmic time steps. That's up to the Synchronizer to and ProcessModels to implement.
  • The Runtime currently misuses the runtime_to_service_cmd channel to send num_steps.
  • The RuntimeService should be split into a RuntimeService and a pure Synchronizer. A universal RuntimeService should service the commands received by the Runtime and then delegate these commands to its Synchronizer which can be different for each RuntimeService.
  • The RunContinuous RunCondition is not supported.
  • The Asynch Synchronizer is currently not thought through. No clear contract between Process and Synchronizer is defined, hence there are numerous ways this can deadlock.

Side question: Should we use asyncio in PyProcModels and PyRuntimeService instead of busy waiting on channels? Will this be more efficient?

@awintel awintel added 1-bug Something isn't working area: magma/runtime Issues with something in lava/magma/runtime labels Nov 21, 2021
@awintel
Copy link
Contributor Author

awintel commented Nov 22, 2021

I started addressing some of these issues.

It should be possible to create a completely generic PyRuntimeService. The responsibility of this PyRuntimeService is merely to communicate RUN, PAUSE, STOP commands between Runtime, and PyProcModels as well as any SET/GET requests. These are generic communication capabilities that are independent of the specific SyncProtocol.

The current LoihiPyRuntimeService conceptually covers these capabilities and can be refactored to become this generic PyRuntimeService except for its run() method.

Only the part of LoihiPyRuntimeService dealing with the different SPK, MGMT, ... phases are specific to a particular SyncProtocol (i.e. the LoihiSyncProtocol) and should therefore be factored out and provided as an object to a refactored PyRuntimeService.

While beginning to refactor LoihiPyRuntimeService, I began to set up a cleaner state machine structure:

  • The PyRuntimeService should have a dictionary mapping commands to function objects, aka 'command handlers'.
  • Each command handler (i.e. what to do when RUN, PAUSE, ... is received) is a separate method of the PyRuntimeService.
  • The core _run(..) method merely iterates over all channels, waits for a command and when it receives one, get and executes the appropriate command handler from the dictionary.
  • The basic PyRuntimeService only has RUN, PAUSE, STOP, GET, SET in its command handler dictionary.
  • Similarly the basic AbstractPyProcessModel would implement a similar state machine with command handlers for commands received from the PyRuntimeService. In addition, any subclass of AbstractPyProcessModel like PyLoihiProcessModel could just add more command->handler mappings to the command handler dictionary of the PyRuntimeService for e.g. handling SPK, PRE_MGMT, etc.

@awintel
Copy link
Contributor Author

awintel commented Nov 22, 2021

This is by far not done, just checked my current experimental state into my private fork to show where I'm going:

https://github.com/awintel/lava/blob/main/src/lava/magma/runtime/runtime.py
https://github.com/awintel/lava/blob/main/src/lava/magma/runtime/runtime_service.py
https://github.com/awintel/lava/blob/main/src/lava/magma/core/model/py/model.py

This has at least enabled me to effectively enable PAUSE, STOP, SET, GET for the AsyncProtocol for a minimalistic use case in this new tutorial:
https://github.com/awintel/lava/blob/main/src/lava/tutorials/end_to_end/tutorial00_tour_through_lava.ipynb

This does currently conflict 99.99% with Harry's changes for performance improvements since we asynchronously started working on the same sections of code in parallel.

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/runtime Issues with something in lava/magma/runtime
Projects
None yet
Development

No branches or pull requests

6 participants