Skip to content

Commit

Permalink
CalcJob: Fix MPI behavior if withmpi option default is True (#6218)
Browse files Browse the repository at this point in the history
The documentation states that if neither:

* the `Code` input (through the `withmpi` attribute)
* the `CalcJob` implementation (through `CodeInfo.withpi')
* nor the `metadata.options.withmpi` input

explicitly defines a boolean value, then the implementation falls back
on the _default_ of the `metadata.options.withmpi` input port. However,
the code was actually ignoring this and always falling back to `False`.
This is now corrected, with `False` still as ultimate fallback value in
case a default for `metadata.options.withmpi` is not defined at all.
  • Loading branch information
sphuber authored Dec 12, 2023
1 parent 83dbe1a commit 8473750
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 26 deletions.
10 changes: 8 additions & 2 deletions aiida/engine/processes/calcjobs/calcjob.py
Original file line number Diff line number Diff line change
Expand Up @@ -946,8 +946,14 @@ def presubmit(self, folder: Folder) -> CalcInfo:
if with_mpi_values_set:
with_mpi = with_mpi_values_set.pop()
else:
# Fall back to the default, which is to not use MPI
with_mpi = False
# Fall back to the default, which is the default of the option in the process input specification with
# ``False`` as final fallback if the default is not even specified
try:
with_mpi = self.spec().inputs['metadata']['options']['withmpi'].default # type: ignore[index]
except RuntimeError:
# ``plumpy.InputPort.default`` raises a ``RuntimeError`` if no default has been set. This is bad
# design and should be changed, but we have to deal with it like this for now.
with_mpi = False

if with_mpi:
prepend_cmdline_params = code.get_prepend_cmdline_params(mpi_args, extra_mpirun_params)
Expand Down
58 changes: 34 additions & 24 deletions tests/engine/processes/calcjobs/test_calc_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,32 +159,34 @@ def prepare_for_submission(self, folder):


@pytest.mark.parametrize(
'code_key, with_mpi_code, with_mpi_option, expected', (
('code_info_with_mpi_none', None, True, 3),
('code_info_with_mpi_none', None, False, 0),
('code_info_with_mpi_none', None, None, 0),
('code_info_with_mpi_none', True, True, 3),
('code_info_with_mpi_none', True, False, None),
('code_info_with_mpi_none', False, True, None),
('code_info_with_mpi_none', False, False, 0),
('code_info_with_mpi_true', None, True, 3),
('code_info_with_mpi_true', None, False, None),
('code_info_with_mpi_true', None, None, 3),
('code_info_with_mpi_true', True, True, 3),
('code_info_with_mpi_true', True, False, None),
('code_info_with_mpi_true', False, True, None),
('code_info_with_mpi_true', False, False, None),
('code_info_with_mpi_false', None, True, None),
('code_info_with_mpi_false', None, False, 0),
('code_info_with_mpi_false', None, None, 0),
('code_info_with_mpi_false', True, True, None),
('code_info_with_mpi_false', True, False, None),
('code_info_with_mpi_false', False, True, None),
('code_info_with_mpi_false', False, False, 0),
'code_key, with_mpi_code, with_mpi_option, with_mpi_option_default, expected', (
('code_info_with_mpi_none', None, True, True, 3),
('code_info_with_mpi_none', None, False, True, 0),
('code_info_with_mpi_none', None, None, False, 0),
('code_info_with_mpi_none', None, None, True, 3),
('code_info_with_mpi_none', True, True, True, 3),
('code_info_with_mpi_none', True, False, True, None),
('code_info_with_mpi_none', False, True, True, None),
('code_info_with_mpi_none', False, False, True, 0),
('code_info_with_mpi_true', None, True, True, 3),
('code_info_with_mpi_true', None, False, True, None),
('code_info_with_mpi_true', None, None, True, 3),
('code_info_with_mpi_true', True, True, True, 3),
('code_info_with_mpi_true', True, False, True, None),
('code_info_with_mpi_true', False, True, True, None),
('code_info_with_mpi_true', False, False, True, None),
('code_info_with_mpi_false', None, True, True, None),
('code_info_with_mpi_false', None, False, True, 0),
('code_info_with_mpi_false', None, None, True, 0),
('code_info_with_mpi_false', True, True, True, None),
('code_info_with_mpi_false', True, False, True, None),
('code_info_with_mpi_false', False, True, True, None),
('code_info_with_mpi_false', False, False, True, 0),
)
)
def test_multi_codes_with_mpi(
aiida_local_code_factory, fixture_sandbox, manager, code_key, with_mpi_code, with_mpi_option, expected
aiida_local_code_factory, fixture_sandbox, manager, code_key, with_mpi_code, with_mpi_option,
with_mpi_option_default, expected
):
"""Test the functionality that controls whether the calculation is to be run with MPI.
Expand Down Expand Up @@ -213,10 +215,18 @@ def test_multi_codes_with_mpi(
}
}

class SubMultiCodesCalcJob(MultiCodesCalcJob):
"""Subclass to override the default for the ``withmpi`` option."""

@classmethod
def define(cls, spec):
super().define(spec)
spec.inputs['metadata']['options']['withmpi'].default = with_mpi_option_default

if with_mpi_option is not None:
inputs['metadata']['options']['withmpi'] = with_mpi_option

process = instantiate_process(manager.get_runner(), MultiCodesCalcJob, **inputs)
process = instantiate_process(manager.get_runner(), SubMultiCodesCalcJob, **inputs)

if expected is None:
with pytest.raises(RuntimeError, match=r'Inconsistent requirements as to whether'):
Expand Down

0 comments on commit 8473750

Please sign in to comment.