search

lincc-frameworks / tape

[Deprecated] Package for working with LSST time series data
https://tape.readthedocs.io
MIT License
12 stars 3 forks source link

Allow Ensemble with no Dask distributed client #211

Closed hombit closed 1 year ago

hombit commented 1 year ago

Currently, there is no simple way to create an Ensemble instance without an associated dask.distributed.Client. While having a client-less Ensemble may not be very useful from a scientific standpoint, it could make Ensemble more lightweight. See issues #208 and #199 for relevant discussions. I've also found that debugging errors with Client is much more difficult, and, for example, pytest-generated errors are much less readable. Therefore, we could probably use a client-less approach in most tests.

I think we can modify Ensemble constructor to accept Client=False or ="no" or something like that.

nevencaplar commented 1 year ago

In general I agree, but I am worried how technically feasible it is. Looking forward to input from Doug and Wilson.

dougbrn commented 1 year ago

+1 to the notion that we should enable clientless setup. How we should represent the API for this is a bit tricky. To align with Dask, client should be able to accept a Dask.distributed.client object, currently if client=None then a client is spun up. The least invasive change could be to have client take in a Dask.distributed.client as a possible input, as before, or also accept a boolean, where client=True spins up a client and passes along kwargs, and client=False doesn't.

dougbrn commented 1 year ago

In the above API, I would still advocate for client=True to be the default behavior

hombit commented 1 year ago

@dougbrn I agree, this issue does not propose changing of the default behavior - just giving one more option

dougbrn commented 1 year ago

Sounds good, I can try taking a stab at this.

hombit commented 1 year ago

Fixed by https://github.com/lincc-frameworks/tape/pull/216