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

Split Environment.run() into two (async) parts - WIP #693

Closed
wants to merge 5 commits into from

Conversation

motus
Copy link
Member

@motus motus commented Feb 29, 2024

Everything is broken atm

@motus motus added WIP Work in progress - do not merge yet mlos-bench labels Feb 29, 2024
@motus motus self-assigned this Feb 29, 2024
@bpkroth
Copy link
Contributor

bpkroth commented Mar 4, 2024

High level design though: why split run at all?

Why not keep the current idea: setup(), run(), teardown() are all synchronous.
That would make it easier to keep the semantics for CompositeEnvs as well, otherwise we need to start expressing which parts of those can be parallelized or not.

Instead, I propose making a new "Worker" concept.

Worker can be assigned an Environment to take through the entire setup, run, teardown phase.

Then, the Scheduler, can have a pool of Workers, that it runs asynchronously, e.g., in separate processes or threads.

See Also #380 for some design note details.

@bpkroth
Copy link
Contributor

bpkroth commented Mar 4, 2024

High level design though: why split run at all?

Why not keep the current idea: setup(), run(), teardown() are all synchronous. That would make it easier to keep the semantics for CompositeEnvs as well, otherwise we need to start expressing which parts of those can be parallelized or not.

Instead, I propose making a new "Worker" concept.

Worker can be assigned an Environment to take through the entire setup, run, teardown phase.

Then, the Scheduler, can have a pool of Workers, that it runs asynchronously, e.g., in separate processes or threads.

See Also #380 for some design note details.

FWIW, I have some async status polling work started that builds off of this idea:

TrialRunner (Worker) class walks an Environment thru the setup/run/teardown phases.
It also gets an EventContextLoop to run background tasks with.
The base run phase is augmented to also start a background async status poll using that EventContextLoop.

I'll try and post a PR when I have something working a little better end-to-end.

@motus
Copy link
Member Author

motus commented Mar 5, 2024

Abandoning in favor of multithreaded wrapper around a synchronous Environment class. Might resume that work later

@motus motus closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlos-bench WIP Work in progress - do not merge yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants