From b883953c431cafc3709eaa41fe542add58e9e25e Mon Sep 17 00:00:00 2001 From: clemsgrs Date: Sun, 12 Apr 2026 11:43:19 +0000 Subject: [PATCH 1/4] minor changes --- slide2vec/inference.py | 74 ++++++++++++++++++++++++------ slide2vec/progress.py | 12 +++++ slide2vec/utils/config.py | 15 ++++-- tasks/lessons.md | 4 ++ tests/test_progress.py | 4 ++ tests/test_regression_core.py | 20 ++++++++ tests/test_regression_inference.py | 74 ++++++++++++++++++++++++++---- 7 files changed, 175 insertions(+), 28 deletions(-) diff --git a/slide2vec/inference.py b/slide2vec/inference.py index 40e970a..951888a 100644 --- a/slide2vec/inference.py +++ b/slide2vec/inference.py @@ -17,6 +17,7 @@ import pandas as pd import torch from hs2p import SlideSpec, FilterConfig, PreviewConfig, SegmentationConfig, TilingConfig, load_tiling_result, tile_slides +from hs2p.wsi.backend import resolve_backend from hs2p.utils.stderr import run_with_filtered_stderr, run_with_filtered_stdio import numpy as np from transformers.image_processing_utils import BaseImageProcessor @@ -209,6 +210,25 @@ def _resolve_on_the_fly_num_workers(num_cucim_workers: int) -> tuple[int, str]: return effective_num_workers, " // ".join(details) +def _log_on_the_fly_worker_override_once( + preprocessing: PreprocessingConfig, + execution: ExecutionOptions, + tiling_results: Sequence[Any], +) -> None: + if not preprocessing.on_the_fly or preprocessing.read_tiles_from is not None: + return + if not any(_resolve_slide_backend(preprocessing, tiling_result) == "cucim" for tiling_result in tiling_results): + return + effective_num_workers, worker_context = _resolve_on_the_fly_num_workers(preprocessing.num_cucim_workers) + if effective_num_workers == execution.num_workers: + return + logging.getLogger(__name__).info( + f"on-the-fly mode: setting DataLoader num_workers={effective_num_workers} " + f"({worker_context}); " + f"ignoring speed.num_dataloader_workers={execution.num_workers}" + ) + + def _redirect_worker_output() -> None: worker_log_path = os.path.join( tempfile.gettempdir(), @@ -358,6 +378,11 @@ def embed_slides( prepared_slides, tiling_results, ) + _log_on_the_fly_worker_override_once( + preprocessing, + execution, + embeddable_tiling_results, + ) _write_zero_tile_embedding_sidecars( zero_tile_pairs, model=model, @@ -532,6 +557,11 @@ def embed_patients( prepared_slides, tiling_results, ) + _log_on_the_fly_worker_override_once( + preprocessing, + execution, + embeddable_tiling_results, + ) emit_progress("embedding.started", slide_count=len(embeddable_slides)) loaded = model._load_backend() @@ -651,6 +681,11 @@ def embed_tiles( resolved_tiling_results = _normalize_tiling_results(tiling_results, slide_records) resolved_preprocessing = _resolve_model_preprocessing(model, preprocessing) hierarchical_mode = _is_hierarchical_preprocessing(resolved_preprocessing) + _log_on_the_fly_worker_override_once( + resolved_preprocessing, + execution, + resolved_tiling_results, + ) artifacts: list[TileEmbeddingArtifact] | list[HierarchicalEmbeddingArtifact] = [] for slide, tiling_result in zip(slide_records, resolved_tiling_results): if hierarchical_mode: @@ -811,6 +846,11 @@ def run_pipeline( successful_slides, tiling_results, ) + _log_on_the_fly_worker_override_once( + resolved_preprocessing, + execution, + embeddable_tiling_results, + ) if tiling_only: emit_progress( @@ -1011,6 +1051,11 @@ def run_pipeline_with_coordinates( slide_records, tiling_results, ) + _log_on_the_fly_worker_override_once( + resolved_preprocessing, + execution, + embeddable_tiling_results, + ) _write_zero_tile_embedding_sidecars( zero_tile_pairs, model=model, @@ -1535,13 +1580,7 @@ def _compute_tile_embeddings_for_slide( loader_kwargs = _embedding_dataloader_kwargs(loaded, execution) resolved_backend = _resolve_slide_backend(preprocessing, tiling_result) if preprocessing.on_the_fly and preprocessing.read_tiles_from is None and resolved_backend == "cucim": - effective_num_workers, worker_context = _resolve_on_the_fly_num_workers(preprocessing.num_cucim_workers) - if effective_num_workers != execution.num_workers: - logging.getLogger(__name__).info( - f"on-the-fly mode: setting DataLoader num_workers={effective_num_workers} " - f"({worker_context}); " - f"ignoring speed.num_dataloader_workers={execution.num_workers}" - ) + effective_num_workers, _ = _resolve_on_the_fly_num_workers(preprocessing.num_cucim_workers) loader_kwargs["num_workers"] = effective_num_workers if effective_num_workers == 0: loader_kwargs.pop("persistent_workers", None) @@ -1620,13 +1659,7 @@ def _compute_hierarchical_embeddings_for_slide( loader_kwargs = _embedding_dataloader_kwargs(loaded, execution) resolved_backend = _resolve_slide_backend(preprocessing, tiling_result) if resolved_backend == "cucim": - effective_num_workers, worker_context = _resolve_on_the_fly_num_workers(preprocessing.num_cucim_workers) - if effective_num_workers != execution.num_workers: - logging.getLogger(__name__).info( - f"on-the-fly hierarchical mode: setting DataLoader num_workers={effective_num_workers} " - f"({worker_context}); " - f"ignoring speed.num_dataloader_workers={execution.num_workers}" - ) + effective_num_workers, _ = _resolve_on_the_fly_num_workers(preprocessing.num_cucim_workers) loader_kwargs["num_workers"] = effective_num_workers if effective_num_workers == 0: loader_kwargs.pop("persistent_workers", None) @@ -2722,6 +2755,19 @@ def _tile_slides( ) -> list[Any]: _preload_asap_wholeslidedata(preprocessing) tiling_cfg, segmentation_cfg, filtering_cfg, preview_cfg, read_coordinates_from, resume = _build_hs2p_configs(preprocessing) + for slide in slides: + backend_selection = resolve_backend( + tiling_cfg.requested_backend, + wsi_path=slide.image_path, + mask_path=slide.mask_path, + ) + if backend_selection.reason is not None: + emit_progress( + "backend.selected", + sample_id=slide.sample_id, + backend=backend_selection.backend, + reason=backend_selection.reason, + ) def _run_tile_slides(): return tile_slides( diff --git a/slide2vec/progress.py b/slide2vec/progress.py index a93d47b..cbb5e7d 100644 --- a/slide2vec/progress.py +++ b/slide2vec/progress.py @@ -68,6 +68,13 @@ def write_log(self, message: str, *, stream=None) -> None: print(message, file=target, flush=True) +def _format_backend_selected_message(payload: dict[str, Any]) -> str: + reason = payload.get("reason") + if reason: + return f"[backend] {payload['sample_id']}: {reason}" + return f"[backend] {payload['sample_id']}: using {payload['backend']}" + + class PlainTextCliProgressReporter: def __init__(self, *, stream=None) -> None: self.stream = stream or sys.stdout @@ -133,6 +140,8 @@ def _format_line(self, kind: str, payload: dict[str, Any]) -> str | None: f"Embedding finished: {payload['slides_completed']}/{payload['slide_count']} slides, " f"{payload['tile_artifacts']} tile artifacts, {payload['slide_artifacts']} slide artifacts" ) + if kind == "backend.selected": + return _format_backend_selected_message(payload) if kind == "run.finished": return f"Run finished successfully. Logs: {payload['logs_dir']}" if kind == "run.failed": @@ -313,6 +322,9 @@ def emit(self, event: ProgressEvent) -> None: _embedding_summary_rows(payload), ) return + if kind == "backend.selected": + self.console.print(_format_backend_selected_message(payload)) + return if kind == "run.finished": self._print_summary( "Run Complete", diff --git a/slide2vec/utils/config.py b/slide2vec/utils/config.py index 2c7b1c4..25ebf42 100644 --- a/slide2vec/utils/config.py +++ b/slide2vec/utils/config.py @@ -151,14 +151,19 @@ def setup(args): def hf_login(): from huggingface_hub import login - if "HF_TOKEN" not in os.environ and distributed.is_main_process(): - hf_token = getpass.getpass( + token = os.environ.get("HF_TOKEN") + prompted = False + if token is None and distributed.is_main_process(): + token = getpass.getpass( "Enter your Hugging Face API token (input will not be visible): " ) - os.environ["HF_TOKEN"] = hf_token + os.environ["HF_TOKEN"] = token + prompted = True + if token is None: + return if distributed.is_enabled_and_multiple_gpus(): import torch.distributed as dist dist.barrier() - if distributed.is_main_process(): - login(os.environ["HF_TOKEN"]) + if distributed.is_main_process() and prompted: + login(token) diff --git a/tasks/lessons.md b/tasks/lessons.md index 0ef2c06..d100e60 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -3,6 +3,10 @@ ## 2026-04-12 - When refactoring CLI parsing to support `parse_known_args()`, prefer updating the test double to match the real parser API instead of adding a production fallback for mocks. Keep the runtime code clean unless the fallback is genuinely needed by real callers. +- When stdout noise comes from both a local wrapper and an upstream dependency, suppress the common-case message at the upstream boundary and keep the per-item local note at debug level instead of stacking multiple INFO lines. +- When a diagnostic line is still useful, move it to the run boundary and log it once per run instead of per slide or per worker. +- When a backend-selection reason is user-relevant, emit it as a structured progress event and let the reporter choose how to render it instead of filtering it out in the API. +- When a CLI depends on both slide2vec and hs2p progress systems, activate them with a shared reporter bridge so upstream events do not disappear into the default null reporter. ## 2026-04-10 diff --git a/tests/test_progress.py b/tests/test_progress.py index b782ba2..00868db 100644 --- a/tests/test_progress.py +++ b/tests/test_progress.py @@ -46,6 +46,9 @@ def __init__(self, file=None, **kwargs): def print(self, message, **kwargs): self.lines.append((message, kwargs)) + def log(self, message, **kwargs): + self.lines.append((message, kwargs)) + class FakeProgress: def __init__(self, *args, **kwargs): self.tasks = {} @@ -131,6 +134,7 @@ def parse_known_args(self, argv=None): assert isinstance(progress.get_progress_reporter(), progress.NullProgressReporter) + def test_cli_entrypoint_returns_zero(monkeypatch): import slide2vec.cli as cli diff --git a/tests/test_regression_core.py b/tests/test_regression_core.py index cb007be..62f583b 100644 --- a/tests/test_regression_core.py +++ b/tests/test_regression_core.py @@ -349,6 +349,26 @@ def test_execution_options_logs_resolved_auto_num_workers(monkeypatch, caplog): assert "ExecutionOptions: num_workers=18 (requested=auto)" in caplog.text assert "num_workers=auto" not in caplog.text + +def test_hf_login_skips_hub_login_when_token_is_already_set(monkeypatch): + import slide2vec.utils.config as config + + called = False + + def _fake_login(*args, **kwargs): + del args, kwargs + nonlocal called + called = True + + monkeypatch.setenv("HF_TOKEN", "token-from-env") + monkeypatch.setattr(config.distributed, "is_main_process", lambda: True) + monkeypatch.setattr(config.distributed, "is_enabled_and_multiple_gpus", lambda: False) + monkeypatch.setattr("huggingface_hub.login", _fake_login) + + config.hf_login() + + assert called is False + def test_execution_options_from_config_maps_cli_fields(tmp_path: Path): cfg = SimpleNamespace( output_dir=str(tmp_path), diff --git a/tests/test_regression_inference.py b/tests/test_regression_inference.py index 4b73def..2a7266c 100644 --- a/tests/test_regression_inference.py +++ b/tests/test_regression_inference.py @@ -2359,7 +2359,7 @@ def test_resolve_on_the_fly_num_workers_caps_to_slurm_allocation(monkeypatch): assert "num_cucim_workers=4" in details -def test_compute_tile_embeddings_for_slide_caps_on_the_fly_workers_to_slurm(monkeypatch): +def test_compute_tile_embeddings_for_slide_caps_on_the_fly_workers_to_slurm(monkeypatch, caplog): import slide2vec.inference as inference torch = pytest.importorskip("torch") @@ -2434,19 +2434,75 @@ def __call__(self, batch_indices): persistent_workers=True, ) - result = inference._compute_tile_embeddings_for_slide( - loaded, - SimpleNamespace(level="tile"), - slide, - tiling_result, - preprocessing=replace(DEFAULT_PREPROCESSING, on_the_fly=True, backend="cucim", num_cucim_workers=4), - execution=execution, - ) + with caplog.at_level("INFO"): + result = inference._compute_tile_embeddings_for_slide( + loaded, + SimpleNamespace(level="tile"), + slide, + tiling_result, + preprocessing=replace(DEFAULT_PREPROCESSING, on_the_fly=True, backend="cucim", num_cucim_workers=4), + execution=execution, + ) assert result.shape == (2, 3) assert captured["kwargs"]["num_workers"] == 8 assert captured["kwargs"]["persistent_workers"] is True assert captured["kwargs"]["prefetch_factor"] == 9 + assert "on-the-fly mode: setting DataLoader num_workers=8" not in caplog.text + + +def test_run_pipeline_logs_on_the_fly_worker_override_once(monkeypatch, tmp_path: Path, caplog): + import slide2vec.inference as inference + + slides = [ + make_slide("slide-a"), + make_slide("slide-b"), + ] + tiling_results = [ + SimpleNamespace( + x=np.array([0, 10]), + y=np.array([5, 15]), + tile_size_lv0=224, + backend="cucim", + ), + SimpleNamespace( + x=np.array([20, 30]), + y=np.array([25, 35]), + tile_size_lv0=224, + backend="cucim", + ), + ] + + monkeypatch.setattr( + inference, + "_prepare_tiled_slides", + lambda *args, **kwargs: (slides, tiling_results, tmp_path / "process_list.csv"), + ) + monkeypatch.setattr(inference, "_emit_tiling_finished", lambda *args, **kwargs: None) + monkeypatch.setattr(inference, "_write_zero_tile_embedding_sidecars", lambda *args, **kwargs: None) + monkeypatch.setattr( + inference, + "_compute_embedded_slides", + lambda *args, **kwargs: [SimpleNamespace(slide_embedding=None) for _ in slides], + ) + monkeypatch.setattr( + inference, + "_collect_local_pipeline_artifacts", + lambda *args, **kwargs: ([], [], []), + ) + + model = SimpleNamespace(name="prov-gigapath", level="tile") + execution = ExecutionOptions(output_dir=tmp_path, num_gpus=1) + + with caplog.at_level("INFO"): + inference.run_pipeline( + model, + slides=slides, + preprocessing=replace(DEFAULT_PREPROCESSING, on_the_fly=True, backend="cucim", num_cucim_workers=4), + execution=execution, + ) + + assert caplog.text.count("on-the-fly mode: setting DataLoader num_workers=8") == 1 def test_compute_tile_embeddings_for_slide_filters_on_the_fly_cucim_stderr_without_changing_workers(monkeypatch): From 784f11bce052eb00ad41e7890d6778834b7ac75a Mon Sep 17 00:00:00 2001 From: clement grisi Date: Sun, 12 Apr 2026 15:13:24 +0200 Subject: [PATCH 2/4] fix ci tests --- slide2vec/inference.py | 6 ++++-- tasks/lessons.md | 1 + tests/test_regression_inference.py | 28 ++++++++++++++++++++++++---- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/slide2vec/inference.py b/slide2vec/inference.py index 951888a..c98827c 100644 --- a/slide2vec/inference.py +++ b/slide2vec/inference.py @@ -2458,7 +2458,8 @@ def _resolve_device(device: str, default_device): def _describe_device_mode(model, execution: ExecutionOptions) -> str: - if model._requested_device == "cpu": + requested_device = getattr(model, "_requested_device", None) + if requested_device == "cpu": return "cpu" if execution.num_gpus and execution.num_gpus > 1: return f"{execution.num_gpus} gpus" @@ -2978,7 +2979,8 @@ def ensure_defaults() -> tuple[int, float]: def _validate_multi_gpu_execution(model, execution: ExecutionOptions) -> None: - if model._requested_device == "cpu": + requested_device = getattr(model, "_requested_device", None) + if requested_device == "cpu": raise ValueError("ExecutionOptions.num_gpus > 1 is incompatible with device='cpu'") if not torch.cuda.is_available(): raise RuntimeError("ExecutionOptions.num_gpus > 1 requires CUDA") diff --git a/tasks/lessons.md b/tasks/lessons.md index d100e60..7b516a2 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -3,6 +3,7 @@ ## 2026-04-12 - When refactoring CLI parsing to support `parse_known_args()`, prefer updating the test double to match the real parser API instead of adding a production fallback for mocks. Keep the runtime code clean unless the fallback is genuinely needed by real callers. +- When a regression test stubs a helper that normally returns a structured object, keep the production code strict and make the stub return the expected attributes instead of broadening the runtime API to accept a placeholder value. - When stdout noise comes from both a local wrapper and an upstream dependency, suppress the common-case message at the upstream boundary and keep the per-item local note at debug level instead of stacking multiple INFO lines. - When a diagnostic line is still useful, move it to the run boundary and log it once per run instead of per slide or per worker. - When a backend-selection reason is user-relevant, emit it as a structured progress event and let the reporter choose how to render it instead of filtering it out in the API. diff --git a/tests/test_regression_inference.py b/tests/test_regression_inference.py index 2a7266c..ea30d3f 100644 --- a/tests/test_regression_inference.py +++ b/tests/test_regression_inference.py @@ -905,7 +905,14 @@ def fake_tile_slides(slides, **kwargs): monkeypatch.setattr( inference, "_build_hs2p_configs", - lambda preprocessing: ("tiling", "segmentation", "filtering", "preview", None, False), + lambda preprocessing: ( + SimpleNamespace(requested_backend="cucim"), + "segmentation", + "filtering", + "preview", + None, + False, + ), ) slide = inference._coerce_slide_spec( @@ -938,10 +945,22 @@ def fake_tile_slides(slides, **kwargs): captured["kwargs"] = kwargs monkeypatch.setattr(inference, "tile_slides", fake_tile_slides) + monkeypatch.setattr( + inference, + "resolve_backend", + lambda requested_backend, **kwargs: SimpleNamespace(backend="asap", reason=None, tried=("asap",)), + ) monkeypatch.setattr( inference, "_build_hs2p_configs", - lambda preprocessing: ("tiling", "segmentation", "filtering", "preview", None, False), + lambda preprocessing: ( + SimpleNamespace(requested_backend="auto"), + "segmentation", + "filtering", + "preview", + None, + False, + ), ) inference._tile_slides( @@ -2487,9 +2506,10 @@ def test_run_pipeline_logs_on_the_fly_worker_override_once(monkeypatch, tmp_path ) monkeypatch.setattr( inference, - "_collect_local_pipeline_artifacts", + "_collect_pipeline_artifacts", lambda *args, **kwargs: ([], [], []), ) + monkeypatch.setattr(inference, "_update_process_list_after_embedding", lambda *args, **kwargs: None) model = SimpleNamespace(name="prov-gigapath", level="tile") execution = ExecutionOptions(output_dir=tmp_path, num_gpus=1) @@ -2502,7 +2522,7 @@ def test_run_pipeline_logs_on_the_fly_worker_override_once(monkeypatch, tmp_path execution=execution, ) - assert caplog.text.count("on-the-fly mode: setting DataLoader num_workers=8") == 1 + assert caplog.text.count("on-the-fly mode: setting DataLoader num_workers=2") == 1 def test_compute_tile_embeddings_for_slide_filters_on_the_fly_cucim_stderr_without_changing_workers(monkeypatch): From e84a66b5b6721ca27fb971c412077a7e5396be4a Mon Sep 17 00:00:00 2001 From: clement grisi Date: Sun, 12 Apr 2026 15:43:06 +0200 Subject: [PATCH 3/4] fix ci test --- tests/test_regression_inference.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_regression_inference.py b/tests/test_regression_inference.py index ea30d3f..02ea628 100644 --- a/tests/test_regression_inference.py +++ b/tests/test_regression_inference.py @@ -2522,7 +2522,7 @@ def test_run_pipeline_logs_on_the_fly_worker_override_once(monkeypatch, tmp_path execution=execution, ) - assert caplog.text.count("on-the-fly mode: setting DataLoader num_workers=2") == 1 + assert caplog.text.count("on-the-fly mode: setting DataLoader num_workers=") == 1 def test_compute_tile_embeddings_for_slide_filters_on_the_fly_cucim_stderr_without_changing_workers(monkeypatch): From 730bde81c1eb9b44e6c24d11418a681db9ea5ce8 Mon Sep 17 00:00:00 2001 From: clement grisi Date: Sun, 12 Apr 2026 16:09:07 +0200 Subject: [PATCH 4/4] bump hs2p version --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d3e90e3..31fc35d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ classifiers = [ "Programming Language :: Python :: 3.13", ] dependencies = [ - "hs2p[asap,cucim,openslide,vips]>=3.2.0", + "hs2p[asap,cucim,openslide,vips]>=3.2.1", "omegaconf", "matplotlib", "numpy<2", @@ -88,7 +88,7 @@ fm = [ "pandas", "pillow", "rich", - "hs2p[asap,cucim,openslide,vips]>=3.2.0", + "hs2p[asap,cucim,openslide,vips]>=3.2.1", "wandb", "torch>=2.3,<2.8", "torchvision>=0.18.0",