lsmo-epfl / aiida-lsmo

AiiDA workflows for the LSMO laboratory at EPFL
Other
9 stars 13 forks source link

SimAnnealing Workchain: Error caused by calcfunction defined as closure #94

Open mpougin opened 3 years ago

mpougin commented 3 years ago

following up on the specific use case described in https://github.com/aiidateam/aiida-core/issues/3304, I was trying to modify the default inputs of the simulated annealing workflow. However, the MC moves are specified in the parameters as param = { ... 'Component': { self.ctx.molecule['name']: { 'MoleculeDefinition': 'Local', 'TranslationProbability': 1.0, 'ReinsertionProbability': 1.0, 'CreateNumberOfMolecules': self.ctx.parameters['number_of_molecules'], }, }, }

My solution is to allow a modification of the ReinsertionProbability and RandomTranslationProbability to make the simulation of charged molecules possible. I left the default parameters as set. I.e. I added the required parameters and modified the component definition: parameters_schema = FF_PARAMETERS_VALIDATOR.extend({ ... Required('reinsertion_probability', default=float(1.0), description='Relative probability to perform a reinsertion move.'): float, Required('randomtranslation_probability', default=float(0.0), description='Relative probability to perform a random translation move.'): float and param = { ... 'ReinsertionProbability': self.ctx.parameters['reinsertion_probability'], 'RandomTranslationProbability': self.ctx.parameters['randomtranslation_probability'], ...

This solution is working fine for my case and doesn't change the default settings. @ltalirz or @danieleongari do you see any problems with this solution? If not, I will create a PR :)

mpougin commented 3 years ago

I just removed my comment in the mentioned issue, as my problem is not directly related to the issue. Here just the quick summary of my issue:

I have a very specific use case: I have a cation (11 atoms in total) with different types of H-atoms, I only have the molecule atoms labeled. I figured out that I can remove the atom number label from the ff by giving arbitrary names to similar atoms with different parameters.

Still, when I try to run the SimAnnealing WC, I run into the issue that the specified settings (https://github.com/lsmo-epfl/aiida-lsmo/blob/ba6360ac83d4000bed266e7678e53331f1115216/aiida_lsmo/workchains/sim_annealing.py#L153-L178) can't be overwritten by my input file, I always receive a ValueError. Do I see this correctly? Background: I also want to set the ReinsertionProbability that is set to 1.0 in the params to 0 (for charged molecules Raspa might run into numerical problems using reinsertion moves, so I want to use random translation moves instead)

ltalirz commented 3 years ago

After a debugging session with Miriam, this is what transpired:

Upon submission of the job script added in PR https://github.com/lsmo-epfl/aiida-lsmo/pull/95 , the error message is

2021-10-20 08:22:52 [52229 | REPORT]: [159960|SimAnnealingWorkChain|on_except]: Traceback (most recent call last):
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/plugins/entry_point.py", line 136, in parse_entry_point_string
    group, name = entry_point_string.split(ENTRY_POINT_STRING_SEPARATOR)
ValueError: not enough values to unpack (expected 2, got 1)

 ...

  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/plugins/entry_point.py", line 138, in parse_entry_point_string
    raise ValueError('invalid entry_point_string format')
ValueError: invalid entry_point_string format
...
During handling of the above exception, another exception occurred:
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/orm/nodes/node.py", line 1275, in <genexpr>
    return (node for node in nodes_identical if node.is_valid_cache)
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/orm/nodes/process/process.py", line 486, in is_valid_cache
    process_class = self.process_class
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/orm/nodes/process/process.py", line 129, in process_class
    process_class = getattr(module, class_name)
AttributeError: module 'aiida_lsmo.workchains.sim_annealing' has no attribute 'get_valid_dict'

The problem is this calcfunction that is defined as a closure (inside setup) which was introduced by me (mea culpa): https://github.com/lsmo-epfl/aiida-lsmo/blob/0999ccec3e445cfd0dfd37a65ab013299a5f7d51/aiida_lsmo/workchains/sim_annealing.py#L201-L204 Replacing this calcfunction by just creating the validate parameters directly inside setup (dirty) got rid of the error message.

To be figured out:

Miriam was running a recent AiiDA version with python 3.8.5