Restore behavior of JobCalculation.get_option returning default val…

…ue (#2013)

Recently, the various methods to get `options` from a `JobCalculation`,
the historically called `get_*` methods, would return a default value
if the value was not explicitly set as an attribute. These methods were
recently replaced by the generic `get_option` method and the
`JobCalculation.options` dictionary, however, by default the `get_option`
would only return a value if explicitly set. One would have to set the
argument `only_actually_set` to `False` to get the default value specified
in `JobCalculation.options`, if it is defined and not `None`.

The business logic however, was always calling `get_option` with default
argument while expecting the default to be returned when not explicitly
set. In addition, certain options whose get methods used to return a
default no longer had a default defined, e.g. `append_text`.

Here we properly define defaults for the options that always had one as
defined by the old explicit get methods. In addition, the default for
the `only_actually_set` for `get_option` is changed to `False`. The
internal code in general always expects to get defaults even when not
set. Note that this required the code for `get_builder_restart` to be
adapted as that expects only those options to be set that were actually
defined by the user when constructing the `JobCalculation`.

aiida/backends/tests/calculation_node.py

@@ -37,6 +37,23 @@ class TestCalcNode(AiidaTestCase):
     emptydict = {}
     emptylist = []
 
+    @classmethod
+    def setUpClass(cls):
+        super(TestCalcNode, cls).setUpClass()
+        from aiida.orm import JobCalculation
+
+        cls.construction_options = {
+            'resources': {
+                'num_machines': 1,
+                'num_mpiprocs_per_machine': 1
+            }
+        }
+
+        cls.job_calculation = JobCalculation()
+        cls.job_calculation.set_computer(cls.computer)
+        cls.job_calculation.set_options(cls.construction_options)
+        cls.job_calculation.store()
+
     def test_process_state(self):
         """
         Check the properties of a newly created bare Calculation
@@ -98,4 +115,30 @@ def test_calculation_updatable_attribute(self):
             a._set_attr(Calculation.PROCESS_STATE_KEY, 'FINISHED')
 
         with self.assertRaises(ModificationNotAllowed):
-            a._del_attr(Calculation.PROCESS_STATE_KEY)
+            a._del_attr(Calculation.PROCESS_STATE_KEY)
+
+    def test_job_calculation_get_option(self):
+        """Verify that options used during calculation construction can be retrieved with `get_option`."""
+        for name, attributes in self.job_calculation.options.items():
+
+            if name in self.construction_options:
+                self.assertEqual(self.job_calculation.get_option(name), self.construction_options[name])
+
+    def test_job_calculation_get_options_only_actually_set(self):
+        """Verify that `get_options only` returns explicitly set options if `only_actually_set=True`."""
+        set_options = self.job_calculation.get_options(only_actually_set=True)
+        self.assertEquals(set(set_options.keys()), set(self.construction_options.keys()))
+
+    def test_job_calculation_get_options_defaults(self):
+        """Verify that `get_options` returns all options with defaults if `only_actually_set=False`."""
+        get_options = self.job_calculation.get_options()
+
+        for name, attributes in self.job_calculation.options.items():
+
+            # If the option was specified in construction options, verify that `get_options` returns same value
+            if name in self.construction_options:
+                self.assertEqual(get_options[name], self.construction_options[name])
+
+            # Otherwise, if the option defines a default that is not `None`, verify that that is returned correctly
+            elif 'default' in attributes and attributes['default'] is not None:
+                self.assertEqual(get_options[name], attributes['default'])

aiida/backends/tests/work/test_process_builder.py

@@ -125,7 +125,7 @@ def test_job_calculation_get_builder_restart(self):
 
         builder = original.get_builder_restart()
 
-        self.assertDictEqual(builder.options, original.get_options())
+        self.assertDictEqual(builder.options, original.get_options(only_actually_set=True))
 
     def test_code_get_builder(self):
         """

aiida/orm/implementation/general/calculation/job/__init__.py

@@ -23,6 +23,7 @@
 from aiida.common.links import LinkType
 from aiida.plugins.loader import get_plugin_type_from_type_string
 from aiida.common.utils import str_timedelta, classproperty
+from aiida.orm.computer import Computer
 from aiida.orm.implementation.general.calculation import AbstractCalculation
 from aiida.orm.mixins import Sealable
 from aiida.utils import timezone
@@ -152,7 +153,7 @@ def get_builder_restart(self):
         from aiida.work.ports import PortNamespace
 
         inputs = self.get_inputs_dict()
-        options = self.get_options()
+        options = self.get_options(only_actually_set=True)
         builder = self.get_builder()
 
         for port_name, port in self.process().spec().inputs.items():
@@ -341,8 +342,6 @@ def _raw_input_folder(self):
         else:
             raise NotExistent("_raw_input_folder not created yet")
 
-    from aiida.orm.computer import Computer
-
     options = {
         'resources': {
             'attribute_key': 'jobresource_params',
@@ -364,6 +363,7 @@ def _raw_input_folder(self):
             'valid_type': six.string_types,
             'non_db': True,
             'required': False,
+            'default': '',
             'help': 'Set a (possibly multiline) string with the commands that the user wants to manually set for the '
                     'scheduler. The difference of this option with respect to the `prepend_text` is the position in '
                     'the scheduler submission file where such text is inserted: with this option, the string is '
@@ -410,6 +410,7 @@ def _raw_input_folder(self):
             'valid_type': (list, tuple),
             'non_db': True,
             'required': False,
+            'default': [],
             'help': 'Set the extra params to pass to the mpirun (or equivalent) command after the one provided in '
                     'computer.mpirun_command. Example: mpirun -np 8 extra_params[0] extra_params[1] ... exec.x',
         },
@@ -448,6 +449,7 @@ def _raw_input_folder(self):
             'valid_type': six.string_types[0],
             'non_db': True,
             'required': False,
+            'default': '',
             'help': 'Set the calculation-specific prepend text, which is going to be prepended in the scheduler-job '
                     'script, just before the code execution',
         },
@@ -456,6 +458,7 @@ def _raw_input_folder(self):
             'valid_type': six.string_types[0],
             'non_db': True,
             'required': False,
+            'default': '',
             'help': 'Set the calculation-specific append text, which is going to be appended in the scheduler-job '
                     'script, just after the code execution',
         },
@@ -468,7 +471,7 @@ def _raw_input_folder(self):
         }
     }
 
-    def get_option(self, name, only_actually_set=True):
+    def get_option(self, name, only_actually_set=False):
         """
         Retun the value of an option that was set for this JobCalculation
 
@@ -512,7 +515,7 @@ def set_option(self, name, value):
 
         self._set_attr(attribute_key, value)
 
-    def get_options(self, only_actually_set=True):
+    def get_options(self, only_actually_set=False):
         """
         Return the dictionary of options set for this JobCalculation
 
@@ -1911,8 +1914,7 @@ def _presubmit(self, folder, use_unstored_links=False):
 
         for k, v in job_tmpl.job_resource.items():
             subst_dict[k] = v
-        mpi_args = [arg.format(**subst_dict) for arg in
-                    computer.get_mpirun_command()]
+        mpi_args = [arg.format(**subst_dict) for arg in computer.get_mpirun_command()]
         extra_mpirun_params = self.get_option('mpirun_extra_params')  # this is the same for all codes in the same calc