From 5011fe4e1ad4bc3c6d02c074c3d0384defbe5fba Mon Sep 17 00:00:00 2001 From: Edward Linscott Date: Wed, 8 Jul 2026 17:53:23 +0200 Subject: [PATCH 1/2] Honour the computer's default_mpiprocs_per_machine in HyperQueueJobResource accepts_default_mpiprocs_per_machine() returned False, so aiida-core's Scheduler.preprocess_resources never injected the computer's default_mpiprocs_per_machine, and the backward-compatibility path num_machines * num_mpiprocs_per_machine fell back to a hard-coded 1: every builder-driven CalcJob on a HyperQueue computer silently ran on a single CPU regardless of the computer's configured default. Return True and handle the None that aiida-core injects when the computer defines no default (falling back to 1 as before). Also join the two halves of the deprecation warning, the second of which was a dead string statement that never reached the message. Co-Authored-By: Claude Fable 5 --- aiida_hyperqueue/scheduler.py | 34 +++++++++++++++++++--------------- tests/test_scheduler.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/aiida_hyperqueue/scheduler.py b/aiida_hyperqueue/scheduler.py index 6c58005..29481c0 100644 --- a/aiida_hyperqueue/scheduler.py +++ b/aiida_hyperqueue/scheduler.py @@ -62,25 +62,24 @@ def validate_resources(cls, **kwargs): try: resources.num_cpus = kwargs.pop("num_cpus") except KeyError: + # For backward compatibility where `num_machines` and `num_mpiprocs_per_machine` + # are set. `num_mpiprocs_per_machine` is normally injected by aiida-core's + # `Scheduler.preprocess_resources` from the computer's + # `default_mpiprocs_per_machine`, which is `None` when the computer defines no + # default — hence the `or`-fallback to 1 rather than a `pop` default. + num_mpiprocs_per_machine = kwargs.pop("num_mpiprocs_per_machine", None) or 1 try: - # For backward compatibility where `num_machines` and `num_mpiprocs_per_machine` are setting - # TODO: I only setting the default value as 1 for `num_mpiprocs_per_machine` because aiida-quantumespresso override - # resources default with `num_machines` set to 1 and then get builder with such setting. - # The `num_mpiprocs_per_machine` sometime can be read from "Default #procs/machine" of computer setup but if it is not exist - # the builder can not be properly get without passing `option` to builder generator. - # It is anyway a workaround for backward compatibility so this default is implemented despite it is quite specific for the qe plugin. - resources.num_cpus = kwargs.pop("num_machines") * kwargs.pop( - "num_mpiprocs_per_machine", 1 - ) + resources.num_cpus = kwargs.pop("num_machines") * num_mpiprocs_per_machine except KeyError: raise KeyError( "Must specify `num_cpus`, or (`num_machines` and `num_mpiprocs_per_machine`)" ) else: - message = "The `num_machines` and `num_mpiprocs_per_machine` for setting hyperqueue resources are deprecated. " - "Please set `num_cpus` and `memory_mb`." - - message = f"{message} (this will be removed in aiida-hyperqueue v1.0)" + message = ( + "The `num_machines` and `num_mpiprocs_per_machine` for setting hyperqueue " + "resources are deprecated. Please set `num_cpus` and `memory_mb`. " + "(this will be removed in aiida-hyperqueue v1.0)" + ) warnings.warn(message, AiiDAHypereQueueDeprecationWarning, stacklevel=3) else: if not isinstance(resources.num_cpus, int): @@ -98,8 +97,13 @@ def validate_resources(cls, **kwargs): @classmethod def accepts_default_mpiprocs_per_machine(cls): - """Return True if this subclass accepts a `default_mpiprocs_per_machine` key, False otherwise.""" - return False + """Return True if this subclass accepts a `default_mpiprocs_per_machine` key, False otherwise. + + Accepting it lets the backward-compatibility `num_machines` path pick up the + computer's ``default_mpiprocs_per_machine`` instead of silently running on a + single CPU. + """ + return True def get_tot_num_mpiprocs(self): """Return the total number of cpus of this job resource.""" diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index b5c47c2..fac1c5c 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -65,6 +65,37 @@ def test_resource_validation(): HyperQueueJobResource(num_cpus=4, memory_mb=1.2) +def test_resource_validation_backward_compatibility(): + """Tests for the deprecated `num_machines` / `num_mpiprocs_per_machine` path.""" + # num_cpus is the product of the two legacy keys + with pytest.warns(Warning, match="deprecated"): + resource = HyperQueueJobResource(num_machines=2, num_mpiprocs_per_machine=8) + assert resource.num_cpus == 16 + + # the computer's default_mpiprocs_per_machine, which aiida-core's + # `Scheduler.preprocess_resources` injects as `num_mpiprocs_per_machine`, + # must be honoured (this is what `accepts_default_mpiprocs_per_machine` + # returning True enables) + with pytest.warns(Warning, match="deprecated"): + resource = HyperQueueJobResource(num_machines=1, num_mpiprocs_per_machine=8) + assert resource.num_cpus == 8 + + # a computer without a default leads aiida-core to inject `None`; fall back to 1 + with pytest.warns(Warning, match="deprecated"): + resource = HyperQueueJobResource(num_machines=1, num_mpiprocs_per_machine=None) + assert resource.num_cpus == 1 + + # `num_mpiprocs_per_machine` omitted entirely also falls back to 1 + with pytest.warns(Warning, match="deprecated"): + resource = HyperQueueJobResource(num_machines=1) + assert resource.num_cpus == 1 + + +def test_accepts_default_mpiprocs_per_machine(): + """The resource class must accept the computer's default so it reaches validate_resources.""" + assert HyperQueueJobResource.accepts_default_mpiprocs_per_machine() + + def test_submit_command(): """Test submit command""" scheduler = HyperQueueScheduler() From c41e1a4e029a780aa8971d2954153cd30a1e54d2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 8 Jul 2026 16:04:11 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiida_hyperqueue/scheduler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aiida_hyperqueue/scheduler.py b/aiida_hyperqueue/scheduler.py index 29481c0..e74412a 100644 --- a/aiida_hyperqueue/scheduler.py +++ b/aiida_hyperqueue/scheduler.py @@ -69,7 +69,9 @@ def validate_resources(cls, **kwargs): # default — hence the `or`-fallback to 1 rather than a `pop` default. num_mpiprocs_per_machine = kwargs.pop("num_mpiprocs_per_machine", None) or 1 try: - resources.num_cpus = kwargs.pop("num_machines") * num_mpiprocs_per_machine + resources.num_cpus = ( + kwargs.pop("num_machines") * num_mpiprocs_per_machine + ) except KeyError: raise KeyError( "Must specify `num_cpus`, or (`num_machines` and `num_mpiprocs_per_machine`)"