Skip to content

dask_num_workers is a no-op: sets pool.num-workers, scheduler reads num_workers (and logs success) #31

Closed

Description

@settylab-dotto-bot

Summary

SampleVarianceEstimator(dask_num_workers=N) configures nothing, and logs
that it succeeded. It writes the Dask config key pool.num-workers, which Dask
does not read; the threaded scheduler reads num_workers.

kompot/differential/sample_variance_estimator.py:513-517:

if self.dask_num_workers is not None:
    dask.config.set(pool=dask.config.get("pool", {}).copy())
    dask.config.set({"pool.num-workers": self.dask_num_workers})
    logger.info(
        f"Configured Dask to use {self.dask_num_workers} workers "
        f"(limits parallelism and memory)"
    )

Measured (dask 2026.8.0):

>>> dask.config.set({"pool.num-workers": 3})
>>> dask.config.get("pool.num-workers")
3
>>> dask.config.get("num_workers")      # what the threaded scheduler reads
None

So the value round-trips through the config dict — which is why this looks
right if you check it that way — and never reaches the scheduler. The log line
then asserts a limit that was not applied.

This is the silent-confident-answer shape: rc 0, nothing on stderr, and a log
message stating the opposite of what happened. Anyone who sets it to bound
memory on a shared node gets no bound and no indication.

Second, smaller issue: it is unreachable from the main API

dask_num_workers is a constructor argument of SampleVarianceEstimator and
is not plumbed through kompot.de(), GPSettings or StorageSettings:

$ grep -rn 'dask_num_workers' kompot/anndata/ kompot/settings.py
$          # no matches

So a de() user — the documented entry point — cannot set it at all, working
or not.

Suggested fix

Set the key the scheduler reads, and prefer scoping it to the computation
rather than mutating global config:

with dask.config.set(num_workers=self.dask_num_workers):
    ...                      # compute inside the context

A global dask.config.set(num_workers=N) would also work but leaks into every
later Dask operation in the same process, which for a library is worse.

If the knob is meant to be usable, plumb it through GPSettings or
StorageSettings so de() can reach it. If it is not, the constructor
argument and its log line are better removed than left asserting a limit that
does not exist.

How it was found

While settling whether a memory figure's run-to-run spread could be removed by
pinning Dask's scheduler pool. It could not — pinning at 1, 4 and the default
(36) all landed inside the same spread — and checking why led here. The
measurement question is unrelated to this defect; the defect is that the knob
Kompot offers for it does nothing.

Pre-existing; not introduced by #29, and not fixed there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions