A/B test jobs

[crusaderky]

• Partially closes Automation of benchmark comparison #292
• Complements A/B test reports #309

This PR adds the ability to run the whole test suite on arbitrary additional coiled software environments.

[crusaderky]

Notably, if #235 will go through we'll lose the ability to pass environment variables.

The alternative to forcing the developer to create an empty file every time was to ignore the file if it doesn't exist, but the risk of misspellings producing unexpected results was too high. Explicit is better than implicit.

[crusaderky]

Ready for final review.
Run with files in AB_environments/: https://github.com/coiled/coiled-runtime/actions/runs/3018905647

[crusaderky]

Question:

In the context of A/B performance tests, do we care about the runtime and stability test folders?
CC @ncclementi @ian-r-rose

[hayesgb]

I can see a solid argument for skipping the runtime folder, but seems to me it would preferable to keep the stability folder in scope.

[fjetter]

full disclosure: I haven't reviewed ab_tests in much detail but rather the broad strokes.

My comments are mostly nitpicks. If there are issues or missing features we can iterate.

Looking forward to see this in action!

[fjetter]

Does there need to be an empty file?

[crusaderky]

Yes. The alternative to forcing the developer to create an empty file every time was to ignore the file if it doesn't exist, but the risk of misspellings quietly resulting in unexpected results was too high. Explicit is better than implicit.

[ntabris]

Why not include empty file with appropriate name in repo? That would mean you'd get no custom config as the default, but also make it easier to not accidentally make a new file w/ wrong name (e.g., you'd either edit existing file or you'd use cp your-file AB_[tab] and autocomplete to correct filename).

[fjetter]

I believe we're now able to ship dask config using the dask.config ctx manager. That might be a better interface than this. is there a reason why we use env vars instead?

[ntabris]

I believe we're now able to ship dask config using the dask.config ctx manager.

Just to be clear, change to ship dask config to cluster is not yet deployed, presumably will go out next week.

[crusaderky]

? This is news to me.
Are you saying that when you call coiled.create_software_environment it will pick up the local config?
Also, I needed something that could be passed to coiled env create. Ideally one would want to pass it a dask config file directly, but I believe there's no such feature?

[ntabris]

Now I'm confused. The feature is that when you create cluster it ships local config. I don't see any connection between dask config and the software environment... am I missing something?

[crusaderky]

Just to be clear, change to ship dask config to cluster is not yet deployed, presumably will go out next week.

This is exactly what we need, please ping me when it's generally available

[crusaderky]

Now I'm confused. The feature is that when you create cluster it ships local config. I don't see any connection between dask config and the software environment... am I missing something?

The script is currently calling coiled env create -e DASK_DISTRIBUTED_...=VALUE

[ntabris]

The script is currently calling coiled env create -e DASK_DISTRIBUTED_...=VALUE

I could be wrong but I think that just applies when creating the software environment and not when you make a cluster using that software environment.

[crusaderky]

It works fine for me?

import coiled
import dask.config
import distributed

!coiled env create --name crusaderky/test_vars --conda AB_environments/AB_baseline.conda.yaml -e DASK_TESTVAR=123
cluster = coiled.Cluster(name="test_vars", n_workers=0, software="crusaderky/test_vars")
client = distributed.Client(cluster)
client.run_on_scheduler(lambda: dask.config.get("testvar"))

123

[ntabris]

Oh, thanks, good to know!