Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input port with default value is validated twice, the second time with a wrong argument #102

Closed
borellim opened this issue May 2, 2019 · 3 comments · Fixed by #106
Closed

Comments

@borellim
Copy link
Member

borellim commented May 2, 2019

(Plumpy 0.11.4 0.13.1 from pip;
aiida-core 1.0.0b2 from commit c7fd2d62d2c;
Python 3.6.8)

I added a validator and a default value to input port 'protocol' of PwBandStructureWorkChain (https://github.com/aiidateam/aiida-quantumespresso/blob/fix-bands-notebook-tutorial-2/aiida_quantumespresso/workflows/pw/band_structure.py)

If I don't pass an input and let it use the default value, the validator function is called twice, the second time with an empty tuple rather than the value to validate (a Dict). If instead I pass a value, the validator function is called once and everything works.

The workchain spec:

class PwBandStructureWorkChain(WorkChain):
    """Workchain to automatically compute a band structure for a given structure using Quantum ESPRESSO pw.x"""

    @classmethod
    def define(cls, spec):
        super(PwBandStructureWorkChain, cls).define(spec)
        spec.input('code', valid_type=orm.Code)
        spec.input('structure', valid_type=orm.StructureData)
        spec.input('protocol', valid_type=orm.Dict, default=orm.Dict(dict={'name':'theos-ht-1.0'}))
                   #validator=_validate_protocol)  # Disable until bug is solved with plumpy
        spec.input('scf_options', valid_type=orm.Dict, default=orm.Dict(dict=get_default_options(with_mpi=True)))
        spec.outline(
            cls.setup_protocol,
            cls.setup_kpoints,
            cls.setup_parameters,
            cls.run_bands,
            cls.run_results,
        )
        spec.exit_code(101, 'ERROR_INVALID_INPUT_UNRECOGNIZED_KIND', message='The bands subworkchain failed')
        spec.exit_code(102, 'ERROR_SUB_PROCESS_FAILED_BANDS', message='the bands PwBasexWorkChain sub process failed')
        spec.output('primitive_structure', valid_type=orm.StructureData)
        spec.output('seekpath_parameters', valid_type=orm.Dict)
        spec.output('scf_parameters', valid_type=orm.Dict)
        spec.output('band_parameters', valid_type=orm.Dict)
        spec.output('band_structure', valid_type=orm.BandsData)

The validator function:

def _validate_protocol(protocol_dict):
   """ Check that the protocol is one for which we have a definition. """
   print("_validate_protocol called")
   print(protocol_dict)
   print(type(protocol_dict))
   try:
       protocol_name = protocol_dict['name']
   except KeyError as e:
       return "Couldn't find key " + str(e) + " in protocol dictionary"
   try:
       protocol = ProtocolManager(protocol_name)
   except ValueError as e:
       return str(e)  # "Unknown protocol '{}'".format(name)

How I run a workchain:

results = launch.run(
    PwBandStructureWorkChain,
    code=code,
    pseudo_family=Str('SSSP_efficiency_1.0'),
    structure=structure_CaF2,
    scf_options = Dict(dict={
            'resources': {
                'num_machines': 1
            },
            'max_wallclock_seconds' : 1800
    }),
    #protocol=Dict(dict={'name':'theos-ht-1.0'})  # <-- this is the relevant input
)

The output:

_validate_protocol called
uuid: 4e7abca1-c56d-45ea-8260-ce4e8f82af57 (unstored)
<class 'aiida.orm.nodes.data.dict.Dict'>
_validate_protocol called
()
<class 'tuple'>

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-fc527d4a8e07> in <module>
     10                 'num_machines': 1
     11             },
---> 12             'max_wallclock_seconds' : 1800
     13     }),
     14     #protocol=Dict(dict={'name':'theos-ht-1.0'})

~/repos/aiida_1.0/aiida-core/aiida/engine/launch.py in run(process, *args, **inputs)
     33         runner = manager.get_manager().get_runner()
     34 
---> 35     return runner.run(process, *args, **inputs)
     36 
     37 

~/repos/aiida_1.0/aiida-core/aiida/engine/runners.py in run(self, process, *args, **inputs)
    204         :return: the outputs of the process
    205         """
--> 206         result, _ = self._run(process, *args, **inputs)
    207         return result
    208 

~/repos/aiida_1.0/aiida-core/aiida/engine/runners.py in _run(self, process, *args, **inputs)
    191 
    192         with utils.loop_scope(self.loop):
--> 193             process = instantiate_process(self, process, *args, **inputs)
    194             process.execute()
    195             return process.outputs, process.node

~/repos/aiida_1.0/aiida-core/aiida/engine/utils.py in instantiate_process(runner, process, *args, **inputs)
     59         raise ValueError('invalid process {}, needs to be Process or ProcessBuilder'.format(type(process)))
     60 
---> 61     process = process_class(runner=runner, inputs=inputs)
     62 
     63     return process

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/state_machine.py in __call__(cls, *args, **kwargs)
    185         """
    186         inst = super(StateMachineMeta, cls).__call__(*args, **kwargs)
--> 187         inst.transition_to(inst.create_initial_state())
    188         call_with_super_check(inst.init)
    189         return inst

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/state_machine.py in transition_to(self, new_state, *args, **kwargs)
    324                 raise
    325             self._transition_failing = True
--> 326             self.transition_failed(initial_state_label, label, *sys.exc_info()[1:])
    327         finally:
    328             self._transition_failing = False

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/state_machine.py in transition_failed(self, initial_state, final_state, exception, trace)
    337         :type exception: :class:`Exception`
    338         """
--> 339         six.reraise(type(exception), exception, trace)
    340 
    341     def get_debug(self):

~/venvs/aiida_py3.6/lib/python3.6/site-packages/six.py in reraise(tp, value, tb)
    691             if value.__traceback__ is not tb:
    692                 raise value.with_traceback(tb)
--> 693             raise value
    694         finally:
    695             value = None

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/state_machine.py in transition_to(self, new_state, *args, **kwargs)
    308 
    309             try:
--> 310                 self._enter_next_state(new_state)
    311             except StateEntryFailed as exception:
    312                 new_state = exception.state

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/state_machine.py in _enter_next_state(self, next_state)
    368     def _enter_next_state(self, next_state):
    369         last_state = self._state
--> 370         self._fire_state_event(StateEventHook.ENTERING_STATE, next_state)
    371         # Enter the new state
    372         next_state.do_enter()

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/state_machine.py in _fire_state_event(self, hook, state)
    286     def _fire_state_event(self, hook, state):
    287         for callback in self._event_callbacks.get(hook, []):
--> 288             callback(self, hook, state)
    289 
    290     @super_check

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/processes.py in <lambda>(_s, _h, state)
    304     def _setup_event_hooks(self):
    305         self.add_state_event_callback(state_machine.StateEventHook.ENTERING_STATE,
--> 306                                       lambda _s, _h, state: self.on_entering(state))
    307         self.add_state_event_callback(state_machine.StateEventHook.ENTERED_STATE,
    308                                       lambda _s, _h, from_state: self.on_entered(from_state))

~/repos/aiida_1.0/aiida-core/aiida/engine/processes/process.py in on_entering(self, state)
    279     @override
    280     def on_entering(self, state):
--> 281         super(Process, self).on_entering(state)
    282         # Update the node attributes every time we enter a new state
    283 

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/processes.py in on_entering(self, state)
    597         state_label = state.LABEL
    598         if state_label == process_states.ProcessState.CREATED:
--> 599             call_with_super_check(self.on_create)
    600         elif state_label == process_states.ProcessState.RUNNING:
    601             call_with_super_check(self.on_run)

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/utils.py in call_with_super_check(fn, *args, **kwargs)
     27     call_count = getattr(self, '_called', 0)
     28     self._called = call_count + 1
---> 29     fn(*args, **kwargs)
     30     assert self._called == call_count, \
     31         "Base '{}' was not called from '{}'\n" \

~/repos/aiida_1.0/aiida-core/aiida/engine/processes/process.py in on_create(self)
    269 
    270     def on_create(self):
--> 271         super(Process, self).on_create()
    272         # If parent PID hasn't been supplied try to get it from the stack
    273         if self._parent_pid is None and Process.current():

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/base/utils.py in new_fn(self, *args, **kwargs)
     14             "The function '{}' was not called through " \
     15             "call_with_super_check".format(fn.__name__)
---> 16         fn(self, *args, **kwargs)
     17         self._called -= 1
     18 

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/processes.py in on_create(self)
    645 
    646         # Input/output
--> 647         self._check_inputs(self._raw_inputs)
    648         raw_inputs = dict(self.raw_inputs) if self.raw_inputs else {}
    649         self._parsed_inputs = self.create_input_args(self.spec().inputs, raw_inputs)

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/processes.py in _check_inputs(self, inputs)
   1230     def _check_inputs(self, inputs):
   1231         # Check the inputs meet the requirements
-> 1232         validation_error = self.spec().validate_inputs(inputs)
   1233         if validation_error:
   1234             raise ValueError(str(validation_error))

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/process_spec.py in validate_inputs(self, inputs)
    180         :rtype: tuple(bool, str or None)
    181         """
--> 182         return self.inputs.validate(inputs)
    183 
    184     def validate_outputs(self, outputs=None):

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/ports.py in validate(self, port_values, breadcrumbs)
    616         # In all other cases, validate all input ports explicitly specified in this port namespace
    617         else:
--> 618             validation_error = self.validate_ports(port_values, breadcrumbs_local)
    619             if validation_error:
    620                 return validation_error

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/ports.py in validate_ports(self, port_values, breadcrumbs)
    638         """
    639         for name, port in self._ports.items():
--> 640             validation_error = port.validate(port_values.pop(name, UNSPECIFIED), breadcrumbs)
    641             if validation_error:
    642                 return validation_error

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/ports.py in validate(self, value, breadcrumbs)
    279         :rtype: typing.Optional[ValidationError]
    280         """
--> 281         validation_error = self._value_spec.validate(value)
    282         if validation_error:
    283             breadcrumbs += (self.name,)

~/venvs/aiida_py3.6/lib/python3.6/site-packages/plumpy/ports.py in validate(self, value)
    148 
    149         if self._validator is not None:
--> 150             result = self._validator(value)
    151             if result is not None:
    152                 assert isinstance(result, str), "Validator returned non string type"

~/repos/aiida_1.0/aiida-quantumespresso/aiida_quantumespresso/workflows/pw/band_structure.py in _validate_protocol(protocol_dict)
     21     #print(protocol_dict.get_dict())
     22     try:
---> 23         protocol_name = protocol_dict['name']
     24     except KeyError as e:
     25         return "Couldn't find key " + str(e) + " in protocol dictionary"

TypeError: tuple indices must be integers or slices, not str
@sphuber
Copy link
Collaborator

sphuber commented May 3, 2019

@borellim you say this concerns plumpy==0.11.4 but didn't you encounter this problem while migrating the tutorial for aiida-core==1.0.0b2? That version should be using plumpy==0.13.1

@borellim
Copy link
Member Author

borellim commented May 3, 2019

Yes, sorry, it's version 0.13.1. I checked the version of plumpy in the wrong environment, but this test was done indeed with aiida-core 1.0.0b2 and plumpy 0.13.1.

@sphuber
Copy link
Collaborator

sphuber commented May 4, 2019

Bug confirmed, problem arises for port validators in combination with defaults. Fix on the way. Thanks for the report @borellim

@sphuber sphuber added this to the v0.13.2 milestone May 4, 2019
borellim added a commit to aiidateam/aiida-quantumespresso that referenced this issue May 15, 2019
We are going to depend on aiida-core 1.0.0b3 very soon, which in turn
depends on plumpy 0.14, which has fixed this issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants