Make with_mpi a strict boolean with default False

[aman-coder03]

this PR makes with_mpi a strict boolean with a default value of False, instead of Optional[bool].

updated:

• data model field to use bool with default False
• constructor to use bool
• getter and setter to only accept bool
• tests to reflect the new behavior

fixes #7082

[aman-coder03]

all the tests pass locally. Please let me know if any changes are needed from my side. Thanks!

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.21%. Comparing base (e79f0a4) to head (24828d6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/aiida/orm/nodes/data/code/abstract.py	60.00%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (e79f0a4) and HEAD (24828d6). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (e79f0a4)	HEAD (24828d6)
	2	1

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7122       +/-   ##
===========================================
- Coverage   79.58%   29.21%   -50.37%     
===========================================
  Files         566      566               
  Lines       43520    43474       -46     
===========================================
- Hits        34631    12695    -21936     
- Misses       8889    30779    +21890     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[edan-bainglass]

Thanks @aman-coder03. In principle, one would think this makes sense (as I did when I came across it). However, I avoided changing this part due to a suspicion that it was Optional for a reason (perhaps historical). As such, good to ask an OG. @giovannipizzi thoughts?

Update

@t-reents found a related PR explaining why this was made Optional. I'll let him describe and link the relevant PR.

[t-reents]

The relevant PR is this one: #6020

As @sphuber explains there, you want to explicitly set a default value (corresponding to something that one could read as "undefined") that is different from an explicit choice of whether MPI is supported or not. If you don't do this, plugin developers would always have to explicitly overwrite it. I think the same applies to the code as well, since AiiDA checks that there are no conflicting values specified.

This is also explained in the relevant part of the code that you can find below:

aiida-core/src/aiida/engine/processes/calcjobs/calcjob.py

Lines 1016 to 1038 in 4e54cd4

	# Here are the three values that will determine whether the code is to be run with MPI _if_ they are not
	# ``None``. If any of them are explicitly defined but are not equivalent, an exception is raised. We use the
	# ``self._raw_inputs`` to determine the actual value passed for ``metadata.options.withmpi`` and
	# distinghuish it from the default.
	raw_inputs = self._raw_inputs or {} # type: ignore[var-annotated]
	with_mpi_option = raw_inputs.get('metadata', {}).get('options', {}).get('withmpi', None)
	with_mpi_plugin = code_info.withmpi
	with_mpi_code = code.with_mpi
	with_mpi_values = [with_mpi_option, with_mpi_plugin, with_mpi_code]
	with_mpi_values_defined = [value for value in with_mpi_values if value is not None]
	with_mpi_values_set = set(with_mpi_values_defined)
	# If more than one value is defined, they have to be identical, or we raise that a conflict is encountered
	if len(with_mpi_values_set) > 1:
	error = f'Inconsistent requirements as to whether code `{code}` should be run with or without MPI.'
	if with_mpi_option is not None:
	error += f'\nThe `metadata.options.withmpi` input was set to `{with_mpi_option}`.'
	if with_mpi_plugin is not None:
	error += f'\nThe plugin require `{with_mpi_plugin}`.'
	if with_mpi_code is not None:
	error += f'\nThe code `{code}` required `{with_mpi_code}`.'
	raise RuntimeError(error)

[aman-coder03]

Thanks a lot for the detailed explanation and for linking the relevant PR. I hadn’t considered the need for an explicit “undefined” (None) state to avoid conflicts between user input, plugin requirements, and code defaults. I’m happy to close this PR as it stands. Really appreciate the clarification!