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

Launcher should support initial_job_num #284

Closed
omry opened this issue Nov 12, 2019 · 2 comments · Fixed by #479
Closed

Launcher should support initial_job_num #284

omry opened this issue Nov 12, 2019 · 2 comments · Fixed by #479
Labels
enhancement Enhanvement request
Milestone

Comments

@omry
Copy link
Collaborator

omry commented Nov 12, 2019

Sweeper plugins may call a launcher multiple times, this means the launcher can no longer know the hydra.job.num without preserving state (which is a bad idea).

Change Launcher launch API to:

def launch(self, job_overrides, initial_idx):

For the BasicSweeper, the change amounts to:

        initial_job_idx = 0
        while not self.is_done():
            batch = self.get_job_batch()
            results = self.launcher.launch(batch, initial_job_idx=initial_job_idx)
            initial_job_idx += len(batch)
            returns.append(results)
        return returns

specifically, see how initial_job_idx is passed to the launcher in each batch, and hen being advanced by the batch size.

@omry omry added the enhancement Enhanvement request label Nov 12, 2019
@omry omry modified the milestones: 0.11.0, 0.12.0 Nov 16, 2019
@omry
Copy link
Collaborator Author

omry commented Mar 8, 2020

@jrapin, this is landing soon.
It solve a problem you may have missed with sweeper plugins that have multiple batches sent to the launcher, where the output directories of the jobs from each batch would overwrite from the previous batch.

It's a pretty easy fix.
there are also some new generic sweeper tests that you may want to try to include.
In particular, see

class BatchedSweeperTestSuite:
    def test_sweep_2_jobs_2_batches(): ...

This function launches 6 jobs and expectes the sweeper to split them into two batches of 3.
See the example sweeper in the plugin to see how to use it. I am not sure it will be easy to integrate with your sweeper but it would be good to try.

@omry
Copy link
Collaborator Author

omry commented Mar 8, 2020

@shagunsodhani , I fixed the Ax sweeper to conform but please try to enable the test. (there is a TODO in the plugin test).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhanvement request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant