SimAnnealing Workchain: Error caused by calcfunction defined as closure

[mpougin]

following up on the specific use case described in aiidateam/aiida-core#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]

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 (

aiida-lsmo/aiida_lsmo/workchains/sim_annealing.py

Lines 153 to 178 in ba6360a

	param = {
	'GeneralSettings': {
	'SimulationType': 'MonteCarlo',
	'NumberOfCycles': self.ctx.parameters['mc_steps'],
	'PrintPropertiesEvery': int(1e10),
	'PrintEvery': self.ctx.parameters['mc_steps'] / 100,
	'RemoveAtomNumberCodeFromLabel':
	True, # BE CAREFULL: needed in AiiDA-1.0.0 because of github.com/aiidateam/aiida-core/issues/3304
	'Forcefield': 'Local',
	'UseChargesFromCIFFile': 'yes',
	'CutOff': self.ctx.parameters['ff_cutoff'],
	},
	'System': {
	'framework_1': {
	'type': 'Framework',
	}
	},
	'Component': {
	self.ctx.molecule['name']: {
	'MoleculeDefinition': 'Local',
	'TranslationProbability': 1.0,
	'ReinsertionProbability': 1.0,
	'CreateNumberOfMolecules': self.ctx.parameters['number_of_molecules'],
	},
	},
	}

) 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]

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

Upon submission of the job script added in PR #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):

aiida-lsmo/aiida_lsmo/workchains/sim_annealing.py

Lines 201 to 204 in 0999cce

	def get_valid_dict(dict_node):
	return Dict(dict=self.parameters_schema(dict_node.get_dict()))
	self.ctx.parameters = get_valid_dict(self.inputs.parameters)

Replacing this calcfunction by just creating the validate parameters directly inside setup (dirty) got rid of the error message.

To be figured out:

• Supposedly, the sim annealing workchain is tested via the https://github.com/lsmo-epfl/aiida-lsmo/blob/develop/examples/test_SimAnnealingWorkChain_MOF74_CO2.py example. Why did this problem not arise there?
  Answer: The issue only occurs when caching is enabled, and only when the cache is already populated.
• Once this is figured out: is there a better way to warn users when they accidentally declare calcfunctions as closures?
  For the moment, we can just continue to allow it Process.process_class raises on calcfunction closure aiidateam/aiida-core#5220
• Some of the settings in ff_data are incorrect (missing commas in atomic_positions). We should add validation to catch this

Miriam was running a recent AiiDA version with python 3.8.5