From b65abf0dcfb49a6d8f3df22093ba1940c5472c81 Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Tue, 18 Aug 2026 20:09:16 +0200 Subject: [PATCH 1/9] fix(experiment): :bug: Detect real parallel launchers, not just any launcher `_is_hydra_parallel()` returned `self.hydra_config.launcher is not None`, but Hydra always populates `hydra.launcher` -- for plain runs as well as multirun -- defaulting to `BasicLauncher`, whose `launch()` is a plain `for` loop over the jobs. The predicate was therefore true for every Hydra job. So both layers deferred to each other: abses assumed Hydra was parallelising and ran its repeats sequentially, while Hydra's BasicLauncher ran the jobs one after another. `Experiment.batch_run(parallels=N)` never reached its `Parallel(backend="loky")` branch under any default Hydra setup, and `exp.num_process` was a dead parameter. The check is now a pure function over the launcher config: a launcher counts as parallel only when it is not one of Hydra's built-in (serial) core plugins. Genuinely concurrent launchers all ship as `hydra_plugins.*` -- joblib, submitit, ray -- so those still make abses yield rather than nest a second process layer inside each Hydra job. The prefix test is broader than matching `basic_launcher` alone: every launcher under `hydra._internal.core_plugins.` is serial, and BasicLauncher is the only one there today, so the prefix keeps holding if Hydra renames it. Existing coverage missed this because `tests/core/test_experiment.py` runs outside Hydra, where `is_hydra_job()` is False and the parallel branch is taken. The new tests pin both directions, and the integration case observes the worker PIDs the repeats actually ran in. Closes #169 Co-Authored-By: Claude Opus 5 --- abses/core/experiment.py | 29 ++++++++++- tests/core/test_experiment.py | 98 ++++++++++++++++++++++++++++++++++- tests/helper.py | 16 ++++++ 3 files changed, 139 insertions(+), 4 deletions(-) diff --git a/abses/core/experiment.py b/abses/core/experiment.py index 9ab16b47..0e55c632 100644 --- a/abses/core/experiment.py +++ b/abses/core/experiment.py @@ -100,6 +100,27 @@ def relative_path_from_to(from_path: Path, to_path: Path) -> Path: ) +# Hydra's built-in launchers live under this prefix and all execute jobs +# sequentially; every genuinely parallel launcher ships as a plugin under +# `hydra_plugins.*`. +_SERIAL_LAUNCHER_PREFIX = "hydra._internal.core_plugins." + + +def launcher_is_parallel(launcher: Any) -> bool: + """Whether a Hydra launcher config actually runs jobs concurrently. + + Hydra always populates `hydra.launcher`, for plain runs as well as for + multirun, defaulting to `BasicLauncher` -- which executes jobs in a plain + `for` loop. "A launcher is configured" therefore says nothing about + concurrency; only a launcher *plugin* (joblib, submitit, ray, ...) runs + jobs in parallel. + """ + if not launcher: + return False + target = str(launcher.get("_target_", "")) + return bool(target) and not target.startswith(_SERIAL_LAUNCHER_PREFIX) + + def run_single( model_cls: Type[MainModelProtocol], cfg: DictConfig, @@ -228,9 +249,13 @@ def cfg(self, cfg: DictConfig | str | Path): self._cfg = cfg def _is_hydra_parallel(self) -> bool: - """检查是否在 Hydra 并行环境中""" + """Whether Hydra is already running jobs in parallel for us. + + When it is, `batch_run` runs its repeats sequentially rather than + nesting a second layer of processes inside each Hydra job. + """ if self.is_hydra_job(): - return self.hydra_config.launcher is not None + return launcher_is_parallel(self.hydra_config.launcher) return False @classmethod diff --git a/tests/core/test_experiment.py b/tests/core/test_experiment.py index 9f139a71..5f6a04cc 100644 --- a/tests/core/test_experiment.py +++ b/tests/core/test_experiment.py @@ -10,11 +10,20 @@ 5. 实验结果的收集 """ +import os +from contextlib import contextmanager +from copy import deepcopy +from pathlib import Path + import pytest +from hydra import compose, initialize +from hydra.core.global_hydra import GlobalHydra +from hydra.core.hydra_config import HydraConfig +from omegaconf import OmegaConf from abses import MainModel -from abses.core.experiment import Experiment -from tests.helper import RandomAddingMod +from abses.core.experiment import Experiment, launcher_is_parallel +from tests.helper import PidReportingMod, RandomAddingMod class TestExperimentBasic: @@ -100,3 +109,88 @@ def test_seed_control(self, test_config): assert results1.equals(results2) # 验证不同种子产生不同结果 assert not results1.equals(results3) + + +class TestLauncherIsParallel: + """`launcher_is_parallel` 只在 launcher 真的并发执行 job 时才为 True。 + + 回归 #169：判据原本是 `launcher is not None`，但 Hydra 单次运行和 + multirun 都默认配一个串行的 BasicLauncher，于是判据恒真， + `Experiment.batch_run(parallels=...)` 永远走不到并行分支。 + """ + + def test_basic_launcher_is_not_parallel(self): + """BasicLauncher 是串行 for 循环，不算并行。""" + launcher = OmegaConf.create( + {"_target_": "hydra._internal.core_plugins.basic_launcher.BasicLauncher"} + ) + assert launcher_is_parallel(launcher) is False + + @pytest.mark.parametrize( + "target", + [ + "hydra_plugins.hydra_joblib_launcher.joblib_launcher.JoblibLauncher", + "hydra_plugins.hydra_submitit_launcher.submitit_launcher.LocalSubmititLauncher", + "hydra_plugins.hydra_submitit_launcher.submitit_launcher.SlurmSubmititLauncher", + "hydra_plugins.hydra_ray_launcher.ray_launcher.RayLauncher", + ], + ) + def test_plugin_launchers_are_parallel(self, target): + """Real launcher plugins do run jobs concurrently, so abses must yield.""" + launcher = OmegaConf.create({"_target_": target}) + assert launcher_is_parallel(launcher) is True + + +@contextmanager +def _inside_hydra_job(launcher_target: str, output_dir: Path): + """Make the process look like a running Hydra job with the given launcher.""" + GlobalHydra.instance().clear() + with initialize(version_base=None, config_path="../config"): + cfg = compose(config_name="test_config.yaml", return_hydra_config=True) + OmegaConf.set_struct(cfg, False) + cfg.hydra.launcher = OmegaConf.create({"_target_": launcher_target}) + cfg.hydra.job.id = 0 + cfg.hydra.runtime.output_dir = str(output_dir) + HydraConfig.instance().set_config(cfg) + try: + yield + finally: + HydraConfig.instance().cfg = None + + +class TestNumProcessInsideHydra: + """`parallels` must still take effect inside a Hydra job (#169). + + Hydra's default BasicLauncher is serial, so abses has to do the + parallelising itself rather than yielding to the launcher. + """ + + @pytest.fixture(autouse=True) + def reset_manager(self): + """Reset the ExperimentManager singleton so a new model class is allowed.""" + from abses.core.job_manager import ExperimentManager + + original = getattr(ExperimentManager, "_instance") + setattr(ExperimentManager, "_instance", None) + yield + setattr(ExperimentManager, "_instance", original) + + def test_repeats_span_multiple_processes(self, test_config, tmp_path): + """Repeats run in worker processes, not all in the parent.""" + cfg = deepcopy(test_config) + cfg.reports.final = {"worker_pid": "worker_pid"} + cfg.outpath = str(tmp_path) + + exp = Experiment.new(PidReportingMod, cfg) + with _inside_hydra_job( + "hydra._internal.core_plugins.basic_launcher.BasicLauncher", + tmp_path, + ): + exp.batch_run(repeats=4, parallels=4, display_progress=False) + + summary = exp.summary() + assert len(summary) == 4, f"expected 4 recorded runs, got {len(summary)}" + + pids = set(summary["worker_pid"]) + assert pids != {os.getpid()}, "every repeat ran in the parent process" + assert len(pids) > 1, f"all repeats shared one process: {pids}" diff --git a/tests/helper.py b/tests/helper.py index e4bd251e..dd8be7e5 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -5,6 +5,8 @@ # GitHub : https://github.com/SongshGeo # Website: https://cv.songshgeo.com/ +import os + from abses import Actor, MainModel @@ -30,3 +32,17 @@ def create_actors_with_metric(model: MainModel, n: int): for i, actor in enumerate(actors): actor.test = float(i) return actors + + +class PidReportingMod(MainModel): + """Reports the OS process it ran in, so parallelism is observable. + + `run_single` executes in a worker process and only its reported vars travel + back, so a final reporter on this attribute is how a test can see which + process each repeat actually ran in. + """ + + @property + def worker_pid(self) -> int: + """PID of the process running this model.""" + return os.getpid() From 29f4ac9715e11d63d6e9ac3f3935efa2fd8fcf5a Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Tue, 18 Aug 2026 20:15:42 +0200 Subject: [PATCH 2/9] fix(experiment): :bug: Keep results from repeats that run in-process The sequential branch of `_batch_run_repeats` called `run_single()` purely for its side effects and dropped the `(key, seed, dataset)` it returns. Only the parallel branch fed those back through `ExperimentManager.update_result()`, so any run that took the sequential path recorded nothing at all and `Experiment.summary()` came back empty. parallels=1 -> summary rows = 0 parallels=2 -> summary rows = 3 parallels=None -> summary rows = 3 This stayed hidden because the default `parallels=None` takes the parallel branch, and it compounded #169: with `_is_hydra_parallel()` true for every Hydra job, *every* run under Hydra lost its summary. Models writing their own output files still produced them, which is why the loss showed up as an empty summary rather than a missing run. The sequential branch now registers each result exactly as the parallel one does. This also makes the yield-to-a-real-launcher direction observable, so the joblib/submitit/ray case finally has an integration test rather than only predicate-level coverage. Co-Authored-By: Claude Opus 5 --- abses/core/experiment.py | 8 +++++++- tests/core/test_experiment.py | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/abses/core/experiment.py b/abses/core/experiment.py index 0e55c632..3bc00ca8 100644 --- a/abses/core/experiment.py +++ b/abses/core/experiment.py @@ -526,7 +526,7 @@ def _batch_run_repeats( # Use print instead of logger to avoid writing to model run log files print(f"Repeat {run_id}: Logging to {log_path}") - run_single( + key, seed, dataset = run_single( model_cls=self.model_cls, cfg=cfg, key=(self.job_id, run_id), @@ -535,6 +535,12 @@ def _batch_run_repeats( hooks=self._manager.hooks, **self._extra_kwargs, ) + self._manager.update_result( + key=key, + datasets=dataset, + seed=seed, + overrides=self.overrides, + ) else: if number_process is None: cpu_count = os.cpu_count() diff --git a/tests/core/test_experiment.py b/tests/core/test_experiment.py index 5f6a04cc..579a938b 100644 --- a/tests/core/test_experiment.py +++ b/tests/core/test_experiment.py @@ -175,6 +175,23 @@ def reset_manager(self): yield setattr(ExperimentManager, "_instance", original) + def test_sequential_path_still_records_results(self, test_config, tmp_path): + """Running the repeats in-process must not lose their results. + + Only the parallel branch used to feed `run_single`'s return value back + to the manager, so any sequential run reported an empty summary. + """ + cfg = deepcopy(test_config) + cfg.reports.final = {"worker_pid": "worker_pid"} + cfg.outpath = str(tmp_path) + + exp = Experiment.new(PidReportingMod, cfg) + exp.batch_run(repeats=3, parallels=1, display_progress=False) + + summary = exp.summary() + assert len(summary) == 3, f"expected 3 recorded runs, got {len(summary)}" + assert set(summary["worker_pid"]) == {os.getpid()} + def test_repeats_span_multiple_processes(self, test_config, tmp_path): """Repeats run in worker processes, not all in the parent.""" cfg = deepcopy(test_config) @@ -194,3 +211,22 @@ def test_repeats_span_multiple_processes(self, test_config, tmp_path): pids = set(summary["worker_pid"]) assert pids != {os.getpid()}, "every repeat ran in the parent process" assert len(pids) > 1, f"all repeats shared one process: {pids}" + + def test_yields_to_a_real_parallel_launcher(self, test_config, tmp_path): + """A launcher plugin already parallelises, so abses must not nest.""" + cfg = deepcopy(test_config) + cfg.reports.final = {"worker_pid": "worker_pid"} + cfg.outpath = str(tmp_path) + + exp = Experiment.new(PidReportingMod, cfg) + with _inside_hydra_job( + "hydra_plugins.hydra_joblib_launcher.joblib_launcher.JoblibLauncher", + tmp_path, + ): + exp.batch_run(repeats=3, parallels=4, display_progress=False) + + summary = exp.summary() + assert len(summary) == 3, f"expected 3 recorded runs, got {len(summary)}" + assert set(summary["worker_pid"]) == {os.getpid()}, ( + "abses nested its own workers inside an already-parallel launcher" + ) From a5599c1b9bccb845e349b4bcf1a0bd0a0ccd156e Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Tue, 18 Aug 2026 20:26:53 +0200 Subject: [PATCH 3/9] refactor(experiment): :recycle: Address code-review findings Quality-only follow-up to the two fixes on this branch; no behaviour change. abses/core/experiment.py: - Both branches of `_batch_run_repeats` recorded results with the same `update_result(...)` block. Extracted `Experiment._record_result()` so a repeat is stored identically whichever branch ran it. - `launcher_is_parallel` took `Any`; its only caller passes the `hydra.launcher` node, so it now says `Optional[DictConfig]`, and it grew the `Args:`/`Returns:` sections the neighbouring module-level helpers all have. - The comment above `_SERIAL_LAUNCHER_PREFIX` claimed every parallel launcher ships under `hydra_plugins.*`. The code tests the converse -- not under `hydra._internal.core_plugins` -- so a third-party *serial* launcher would be read as parallel. Reworded to state what is actually checked, and why erring that way is cheap. tests: - The ExperimentManager singleton save/reset/restore existed twice in `test_experiment.py`; it is now the `reset_experiment_manager` fixture in `conftest.py`, used via `pytestmark`. The third copy in `tests/utils/test_logging.py` is left alone: it is interleaved with logger handler teardown in the same `try/finally`, so moving it is riskier than the duplication it would remove. - The three Hydra tests repeated the same config arrangement; it is now the `pid_config` fixture. - `TestLauncherIsParallel` mixed Chinese and English docstrings within the one new class; it is English throughout now, matching the class beside it. Co-Authored-By: Claude Opus 5 --- abses/core/experiment.py | 72 ++++++++++++++++++++++------------- tests/conftest.py | 15 ++++++++ tests/core/test_experiment.py | 64 ++++++++++--------------------- 3 files changed, 81 insertions(+), 70 deletions(-) diff --git a/abses/core/experiment.py b/abses/core/experiment.py index 3bc00ca8..c35f65ef 100644 --- a/abses/core/experiment.py +++ b/abses/core/experiment.py @@ -100,20 +100,27 @@ def relative_path_from_to(from_path: Path, to_path: Path) -> Path: ) -# Hydra's built-in launchers live under this prefix and all execute jobs -# sequentially; every genuinely parallel launcher ships as a plugin under -# `hydra_plugins.*`. +# Hydra ships exactly one built-in launcher, `BasicLauncher`, and it is serial. +# Anything outside this prefix is a third-party launcher plugin, which we treat +# as parallel: the launchers that exist in practice (joblib, submitit, ray) all +# are, and assuming parallel only costs us a layer of nesting we skip. _SERIAL_LAUNCHER_PREFIX = "hydra._internal.core_plugins." -def launcher_is_parallel(launcher: Any) -> bool: +def launcher_is_parallel(launcher: Optional[DictConfig]) -> bool: """Whether a Hydra launcher config actually runs jobs concurrently. Hydra always populates `hydra.launcher`, for plain runs as well as for multirun, defaulting to `BasicLauncher` -- which executes jobs in a plain `for` loop. "A launcher is configured" therefore says nothing about - concurrency; only a launcher *plugin* (joblib, submitit, ray, ...) runs - jobs in parallel. + concurrency. + + Args: + launcher: The `hydra.launcher` config node, if there is one. + + Returns: + True when the launcher is a plugin that runs jobs in parallel, so the + caller should not add a second layer of processes of its own. """ if not launcher: return False @@ -415,6 +422,26 @@ def _get_seed(self, run_id: int, job_id: Optional[int] = None) -> Optional[int]: r = random.Random(self._base_seed + job_id * 1000 + run_id) return r.randrange(2**32) + def _record_result( + self, result: Tuple[Tuple[int, int], Optional[int], pd.DataFrame] + ) -> None: + """Register what one `run_single` call produced. + + Both the sequential and the parallel branch of `_batch_run_repeats` + record through here, so a repeat is stored the same way whichever + branch ran it. + + Args: + result: The `(key, seed, datasets)` triple `run_single` returns. + """ + key, seed, datasets = result + self._manager.update_result( + key=key, + datasets=datasets, + seed=seed, + overrides=self.overrides, + ) + def _get_logging_mode(self) -> str: """Get logging mode from experiment configuration. @@ -526,20 +553,16 @@ def _batch_run_repeats( # Use print instead of logger to avoid writing to model run log files print(f"Repeat {run_id}: Logging to {log_path}") - key, seed, dataset = run_single( - model_cls=self.model_cls, - cfg=cfg, - key=(self.job_id, run_id), - outpath=self.outpath, - seed=self._get_seed(run_id), - hooks=self._manager.hooks, - **self._extra_kwargs, - ) - self._manager.update_result( - key=key, - datasets=dataset, - seed=seed, - overrides=self.overrides, + self._record_result( + run_single( + model_cls=self.model_cls, + cfg=cfg, + key=(self.job_id, run_id), + outpath=self.outpath, + seed=self._get_seed(run_id), + hooks=self._manager.hooks, + **self._extra_kwargs, + ) ) else: if number_process is None: @@ -568,13 +591,8 @@ def _batch_run_repeats( ) ) # 在主进程中批量更新结果 - for key, seed, dataset in results: - self._manager.update_result( - key=key, - datasets=dataset, - seed=seed, - overrides=self.overrides, - ) + for result in results: + self._record_result(result) def batch_run( self, diff --git a/tests/conftest.py b/tests/conftest.py index 5d5fab89..d45cd39a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -176,3 +176,18 @@ def mock_points_gdf(): "geometry": [Point(0, 0), Point(1, 1), Point(2, 2)], } return gpd.GeoDataFrame(data, crs="epsg:4326") + + +@pytest.fixture(name="reset_experiment_manager") +def reset_experiment_manager(): + """Reset the ExperimentManager singleton between tests. + + The manager is a singleton pinned to the first model class it sees, so a + test that experiments on a different model class has to clear it first. + """ + from abses.core.job_manager import ExperimentManager + + original = getattr(ExperimentManager, "_instance", None) + ExperimentManager._instance = None + yield + ExperimentManager._instance = original diff --git a/tests/core/test_experiment.py b/tests/core/test_experiment.py index 579a938b..f6659677 100644 --- a/tests/core/test_experiment.py +++ b/tests/core/test_experiment.py @@ -77,18 +77,7 @@ def test_parameter_override(self, test_config): class TestExperimentRandom: """测试实验的随机性控制""" - @pytest.fixture(autouse=True) - def setup_class(self): - """在每个测试类运行前重置实验管理器""" - from abses.core.job_manager import ExperimentManager - - # 保存当前的实例 - self._original_instance = getattr(ExperimentManager, "_instance") - # 清空实例 - setattr(ExperimentManager, "_instance", None) - yield - # 测试结束后恢复原来的实例 - setattr(ExperimentManager, "_instance", self._original_instance) + pytestmark = pytest.mark.usefixtures("reset_experiment_manager") def test_seed_control(self, test_config): """测试随机种子控制""" @@ -112,15 +101,16 @@ def test_seed_control(self, test_config): class TestLauncherIsParallel: - """`launcher_is_parallel` 只在 launcher 真的并发执行 job 时才为 True。 + """`launcher_is_parallel` is True only when the launcher really is concurrent. - 回归 #169：判据原本是 `launcher is not None`，但 Hydra 单次运行和 - multirun 都默认配一个串行的 BasicLauncher，于是判据恒真， - `Experiment.batch_run(parallels=...)` 永远走不到并行分支。 + Regression for #169: the check used to be `launcher is not None`, but Hydra + configures a serial BasicLauncher for plain runs and multirun alike, so it + was always true and `Experiment.batch_run(parallels=...)` never reached its + parallel branch. """ def test_basic_launcher_is_not_parallel(self): - """BasicLauncher 是串行 for 循环，不算并行。""" + """BasicLauncher runs jobs in a `for` loop, which is not parallel.""" launcher = OmegaConf.create( {"_target_": "hydra._internal.core_plugins.basic_launcher.BasicLauncher"} ) @@ -165,40 +155,32 @@ class TestNumProcessInsideHydra: parallelising itself rather than yielding to the launcher. """ - @pytest.fixture(autouse=True) - def reset_manager(self): - """Reset the ExperimentManager singleton so a new model class is allowed.""" - from abses.core.job_manager import ExperimentManager + pytestmark = pytest.mark.usefixtures("reset_experiment_manager") - original = getattr(ExperimentManager, "_instance") - setattr(ExperimentManager, "_instance", None) - yield - setattr(ExperimentManager, "_instance", original) + @pytest.fixture(name="pid_config") + def pid_reporting_config(self, test_config, tmp_path): + """A config whose only final report is the process each repeat ran in.""" + cfg = deepcopy(test_config) + cfg.reports.final = {"worker_pid": "worker_pid"} + cfg.outpath = str(tmp_path) + return cfg - def test_sequential_path_still_records_results(self, test_config, tmp_path): + def test_sequential_path_still_records_results(self, pid_config): """Running the repeats in-process must not lose their results. Only the parallel branch used to feed `run_single`'s return value back to the manager, so any sequential run reported an empty summary. """ - cfg = deepcopy(test_config) - cfg.reports.final = {"worker_pid": "worker_pid"} - cfg.outpath = str(tmp_path) - - exp = Experiment.new(PidReportingMod, cfg) + exp = Experiment.new(PidReportingMod, pid_config) exp.batch_run(repeats=3, parallels=1, display_progress=False) summary = exp.summary() assert len(summary) == 3, f"expected 3 recorded runs, got {len(summary)}" assert set(summary["worker_pid"]) == {os.getpid()} - def test_repeats_span_multiple_processes(self, test_config, tmp_path): + def test_repeats_span_multiple_processes(self, pid_config, tmp_path): """Repeats run in worker processes, not all in the parent.""" - cfg = deepcopy(test_config) - cfg.reports.final = {"worker_pid": "worker_pid"} - cfg.outpath = str(tmp_path) - - exp = Experiment.new(PidReportingMod, cfg) + exp = Experiment.new(PidReportingMod, pid_config) with _inside_hydra_job( "hydra._internal.core_plugins.basic_launcher.BasicLauncher", tmp_path, @@ -212,13 +194,9 @@ def test_repeats_span_multiple_processes(self, test_config, tmp_path): assert pids != {os.getpid()}, "every repeat ran in the parent process" assert len(pids) > 1, f"all repeats shared one process: {pids}" - def test_yields_to_a_real_parallel_launcher(self, test_config, tmp_path): + def test_yields_to_a_real_parallel_launcher(self, pid_config, tmp_path): """A launcher plugin already parallelises, so abses must not nest.""" - cfg = deepcopy(test_config) - cfg.reports.final = {"worker_pid": "worker_pid"} - cfg.outpath = str(tmp_path) - - exp = Experiment.new(PidReportingMod, cfg) + exp = Experiment.new(PidReportingMod, pid_config) with _inside_hydra_job( "hydra_plugins.hydra_joblib_launcher.joblib_launcher.JoblibLauncher", tmp_path, From ff226bc423218ae525e3f24be2020a096f22abe7 Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Tue, 18 Aug 2026 20:30:25 +0200 Subject: [PATCH 4/9] refactor(experiment): :recycle: Spell out the default process count `max(1, cpu_count or 1 // 2)` reads as "half the cores", but `1 // 2` binds first and evaluates to 0, so `cpu_count or 0` is just `cpu_count`: the default has always been every core, capped at `repeats` on the next line. Left at every core deliberately rather than "fixed" to half -- that is the behaviour every released version has had for `parallels=None`, and halving it would quietly double the wall time of existing runs. This matters more now than it did: with #169 fixed the parallel branch is reachable under Hydra, so a Hydra user who never set `num_process` goes from one process to all cores. Worth a line in the release notes. Co-Authored-By: Claude Opus 5 --- abses/core/experiment.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/abses/core/experiment.py b/abses/core/experiment.py index c35f65ef..44e7a55f 100644 --- a/abses/core/experiment.py +++ b/abses/core/experiment.py @@ -566,8 +566,13 @@ def _batch_run_repeats( ) else: if number_process is None: - cpu_count = os.cpu_count() - number_process = max(1, cpu_count or 1 // 2) + # `or 1` covers os.cpu_count() returning None on exotic + # platforms. This used to read `cpu_count or 1 // 2`, where + # `1 // 2` binds first and evaluates to 0, so the default has + # always been every core rather than half of them; spelling it + # out keeps that behaviour instead of silently halving it. + cpu_count = os.cpu_count() or 1 + number_process = max(1, cpu_count) number_process = min(number_process, repeats) results = Parallel( From 2538d0af065f881c0756600f55937d0b3f591ed4 Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Tue, 18 Aug 2026 21:32:00 +0200 Subject: [PATCH 5/9] chore(ci): :construction_worker: Make interpreter crashes visible, back-merge dev automatically Two CI changes, no source changes. **Diagnostics for #171.** `Tests 3.13 on windows-latest` has been failing for seven months with `Error -1073741819` (0xC0000005, ACCESS_VIOLATION) and no Python output whatsoever -- pytest crashes before it prints its banner, so there is nothing to go on. `PYTHONFAULTHANDLER=1` on the job makes the interpreter dump a C-level traceback on a fatal signal, and a separate import smoke test before `make test-all` tells apart "importing abses crashes" from "the test run crashes", which the single combined step cannot. Neither change tries to fix the crash -- it does not reproduce off Windows. They exist so the next red run says where it died. **Automatic back-merge.** release-please commits the version bump and changelog on `master` only, and nothing brought them back, so `dev` sat at 0.10.0 while 0.11.7 was already on PyPI -- which is why `abses.__version__` read 0.10.0 from a dev checkout. A new `backmerge-dev` job opens master -> dev after each release. It no-ops when dev is already up to date or a back-merge PR is already open. Co-Authored-By: Claude Opus 5 --- .github/workflows/release-please.yml | 46 ++++++++++++++++++++++++++++ .github/workflows/tests.yml | 11 +++++++ 2 files changed, 57 insertions(+) diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 3eac0a13..b68cd83a 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -53,3 +53,49 @@ jobs: run: | uv build uv publish --token ${{ secrets.PYPI_TOKEN }} + + # release-please only bumps the version and changelog on master. Without this + # dev keeps whatever version it had -- it sat at 0.10.0 while 0.11.7 was on + # PyPI, so `abses.__version__` read 0.10.0 from a dev checkout. + backmerge-dev: + needs: release-please + if: ${{ needs.release-please.outputs.release_created }} + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Open a PR merging the release back into dev + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TAG: ${{ needs.release-please.outputs.tag_name }} + run: | + set -euo pipefail + git fetch origin master dev + + if [ "$(git rev-list --count origin/dev..origin/master)" -eq 0 ]; then + echo "dev already contains master; nothing to back-merge." + exit 0 + fi + + existing=$(gh pr list --base dev --head master --state open \ + --json number --jq '.[0].number // empty') + if [ -n "$existing" ]; then + echo "Back-merge PR already open: #$existing" + exit 0 + fi + + gh pr create --base dev --head master \ + --title "chore(release): merge ${TAG} back into dev" \ + --body "Automated back-merge after ${TAG}. + + release-please commits the version bump and changelog on \`master\` only, + so \`dev\` needs this to stay in step. Merging is safe to do without review + when the diff is just \`pyproject.toml\` and \`CHANGELOG.md\`. + + Note: PRs opened by \`GITHUB_TOKEN\` do not trigger workflows, so this one + starts with no checks. Close and reopen it if you want CI to run." diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a36f9edb..f296fed5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -45,6 +45,11 @@ jobs: os: [ubuntu-latest, macos-latest, windows-latest] python-version: ["3.11", "3.12", "3.13"] + # A hard interpreter crash (see #171) is silent without this: the job just + # reports an exit code and no Python traceback at all. + env: + PYTHONFAULTHANDLER: "1" + steps: - uses: actions/checkout@v4 @@ -68,6 +73,12 @@ jobs: run: | uv sync --dev + # Separates "importing abses crashes" from "the test run crashes", which + # the combined `make test-all` step cannot distinguish -- see #171. + - name: Import smoke test + run: | + uv run python -X faulthandler -c "import abses; print(abses.__version__)" + - name: Run Make test-all (unit + notebooks + tox) run: | uv run make test-all From d42a60ce57d016f98cd8c2709f72a891b27a5064 Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Tue, 18 Aug 2026 23:30:47 +0200 Subject: [PATCH 6/9] fix(tests): :white_check_mark: Stop asserting joblib's dispatch timing `test_repeats_span_multiple_processes` asserted the four repeats landed on more than one worker PID. That is joblib's dispatch timing, not a promise this code makes: the runs are short enough that one worker can take all four before the others have finished starting, which is what happened on the macOS 3.11 runner: AssertionError: all repeats shared one process: {2078} PID 2078 was not the parent, so the parallel branch had worked exactly as intended -- the assertion was simply testing something the implementation never guaranteed. What actually separates the two branches is the parent process: the sequential branch runs there and loky never does. The test now asserts the repeats did not run in the parent, and is renamed for what it checks. This does not weaken the regression. Restoring the #169 bug still fails it: AssertionError: repeats ran in the parent, so the parallel branch was skipped: {18952} Ran 10 consecutive times against the fix with no flake; full suite 707 passed. Co-Authored-By: Claude Opus 5 --- tests/core/test_experiment.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/core/test_experiment.py b/tests/core/test_experiment.py index f6659677..17a6e236 100644 --- a/tests/core/test_experiment.py +++ b/tests/core/test_experiment.py @@ -178,8 +178,8 @@ def test_sequential_path_still_records_results(self, pid_config): assert len(summary) == 3, f"expected 3 recorded runs, got {len(summary)}" assert set(summary["worker_pid"]) == {os.getpid()} - def test_repeats_span_multiple_processes(self, pid_config, tmp_path): - """Repeats run in worker processes, not all in the parent.""" + def test_repeats_runs_in_worker_processes(self, pid_config, tmp_path): + """Repeats run in worker processes rather than in the parent.""" exp = Experiment.new(PidReportingMod, pid_config) with _inside_hydra_job( "hydra._internal.core_plugins.basic_launcher.BasicLauncher", @@ -190,9 +190,15 @@ def test_repeats_span_multiple_processes(self, pid_config, tmp_path): summary = exp.summary() assert len(summary) == 4, f"expected 4 recorded runs, got {len(summary)}" + # Whether the repeats land on one worker or four is joblib's dispatch + # timing, not a promise this code makes: these runs are short enough + # that one worker can take all four before the others have started. + # What distinguishes the two branches is the parent process, and loky + # never executes in it. pids = set(summary["worker_pid"]) - assert pids != {os.getpid()}, "every repeat ran in the parent process" - assert len(pids) > 1, f"all repeats shared one process: {pids}" + assert os.getpid() not in pids, ( + f"repeats ran in the parent, so the parallel branch was skipped: {pids}" + ) def test_yields_to_a_real_parallel_launcher(self, pid_config, tmp_path): """A launcher plugin already parallelises, so abses must not nest.""" From 46a4ae1efee7d4c874360d4ae6d6816930fcb1a1 Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Wed, 19 Aug 2026 17:01:10 +0200 Subject: [PATCH 7/9] fix(experiment): :bug: Respect a Joblib launcher pinned to one job `launcher_is_parallel` classified any non-builtin launcher as concurrent from its `_target_` alone. joblib's `Parallel(n_jobs=1)` selects the sequential backend and runs every job in the calling process, so a run with `hydra/launcher=joblib hydra.launcher.n_jobs=1` was serial on both levels: Hydra ran the jobs one at a time and `batch_run` skipped its parallel branch believing Hydra had it covered. `parallels` was ignored exactly as in #169, only in a narrower configuration. The check now reads `n_jobs` as well. joblib is the only launcher that spells its concurrency knob that way; every other value, the -1 default and an absent key all leave the launcher classified as parallel. An `n_jobs` that cannot be resolved keeps that optimistic default rather than raising out of a predicate and aborting the run. Reported by CodeRabbit on #173. --- abses/core/experiment.py | 21 ++++++++++++++++++--- tests/core/test_experiment.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/abses/core/experiment.py b/abses/core/experiment.py index 44e7a55f..7c58ef81 100644 --- a/abses/core/experiment.py +++ b/abses/core/experiment.py @@ -40,6 +40,7 @@ from hydra.core.hydra_config import HydraConf, HydraConfig from joblib import Parallel, delayed from omegaconf import DictConfig, OmegaConf +from omegaconf.errors import OmegaConfBaseException from tqdm.auto import tqdm from abses.core.job_manager import ExperimentManager @@ -102,8 +103,9 @@ def relative_path_from_to(from_path: Path, to_path: Path) -> Path: # Hydra ships exactly one built-in launcher, `BasicLauncher`, and it is serial. # Anything outside this prefix is a third-party launcher plugin, which we treat -# as parallel: the launchers that exist in practice (joblib, submitit, ray) all -# are, and assuming parallel only costs us a layer of nesting we skip. +# as parallel unless its own config says otherwise: the launchers that exist in +# practice (joblib, submitit, ray) all are, and assuming parallel only costs us +# a layer of nesting we skip. _SERIAL_LAUNCHER_PREFIX = "hydra._internal.core_plugins." @@ -125,7 +127,20 @@ def launcher_is_parallel(launcher: Optional[DictConfig]) -> bool: if not launcher: return False target = str(launcher.get("_target_", "")) - return bool(target) and not target.startswith(_SERIAL_LAUNCHER_PREFIX) + if not target or target.startswith(_SERIAL_LAUNCHER_PREFIX): + return False + # A plugin can still be told to run serially. `n_jobs` is joblib's knob and + # joblib is the only launcher that spells it that way; `n_jobs: 1` selects + # joblib's sequential backend, which runs every job in the calling process + # exactly like BasicLauncher does. Other values -- including the -1 default + # and an absent key -- leave the launcher concurrent. + try: + n_jobs = OmegaConf.select(launcher, "n_jobs", default=None) + except OmegaConfBaseException: + # An unreadable value tells us nothing; keep the optimistic default + # rather than aborting the run from inside a predicate. + return True + return n_jobs != 1 def run_single( diff --git a/tests/core/test_experiment.py b/tests/core/test_experiment.py index 17a6e236..be7dda6e 100644 --- a/tests/core/test_experiment.py +++ b/tests/core/test_experiment.py @@ -130,6 +130,36 @@ def test_plugin_launchers_are_parallel(self, target): launcher = OmegaConf.create({"_target_": target}) assert launcher_is_parallel(launcher) is True + def test_joblib_with_one_job_is_not_parallel(self): + """`n_jobs: 1` makes joblib run every job in the calling process. + + joblib's `Parallel(n_jobs=1)` uses the sequential backend, so this + launcher is as serial as BasicLauncher despite being a plugin. + """ + launcher = OmegaConf.create( + { + "_target_": "hydra_plugins.hydra_joblib_launcher" + ".joblib_launcher.JoblibLauncher", + "n_jobs": 1, + } + ) + assert launcher_is_parallel(launcher) is False + + def test_unreadable_n_jobs_falls_back_to_parallel(self): + """An `n_jobs` we cannot read must not crash the run. + + This is a predicate on the way to `batch_run`; raising here would abort + the whole experiment. Assuming parallel only costs a layer of nesting. + """ + launcher = OmegaConf.create( + { + "_target_": "hydra_plugins.hydra_joblib_launcher" + ".joblib_launcher.JoblibLauncher", + "n_jobs": "${undefined_key}", + } + ) + assert launcher_is_parallel(launcher) is True + @contextmanager def _inside_hydra_job(launcher_target: str, output_dir: Path): From 250b0e22d24a1cdc5061979b90d6dcba650ed7ff Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Wed, 19 Aug 2026 17:01:11 +0200 Subject: [PATCH 8/9] chore(ci): :lock: Narrow the back-merge job to read access on contents The job checks out, fetches, and opens a pull request against a branch that already exists on the remote; it never pushes. `contents: read` is enough for the first two and `pull-requests: write` covers the third. Reported by CodeRabbit on #173. --- .github/workflows/release-please.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index b68cd83a..a6b84cee 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -62,7 +62,7 @@ jobs: if: ${{ needs.release-please.outputs.release_created }} runs-on: ubuntu-latest permissions: - contents: write + contents: read pull-requests: write steps: - uses: actions/checkout@v4 From 02629602ad701b75377b99cfb02dc8db93fd19a4 Mon Sep 17 00:00:00 2001 From: SongshGeo Date: Wed, 19 Aug 2026 17:01:11 +0200 Subject: [PATCH 9/9] chore: :lock: Sync the lockfile to the released version `uv.lock` still recorded abses 0.10.0, left over from the version drift between dev and master. Any `uv run` regenerated it and dirtied the working tree. --- uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index fd5c1e09..3197164d 100644 --- a/uv.lock +++ b/uv.lock @@ -8,7 +8,7 @@ resolution-markers = [ [[package]] name = "abses" -version = "0.10.0" +version = "0.11.7" source = { editable = "." } dependencies = [ { name = "fiona" },