workflow untested with aiida-core 1.0

[danieleongari]

I'm using and testing it:

• the ha parameter does not work anymore with the value True (should be that same as DEF, but practically it is not printing anything in the cml after -ha). This needs to be restored for simplicity and also it should be the default, since non-ha calculations are very inaccurate in Zeopp. I guess also ha: False is not working.
• I would remove the workflow blockpocket since it is not compatible with AiiDA-1 and it is anyway of little use: if one wants to include the calcualtion of the blocking spheres in e.g., a MC simulation, it is simpler to include it in that workchain instead of nesting workchains. In general I don't see the need of including workchains in the plugin of zeopp, being such a simple code.

[ezpzbz]

• the ha parameter does not work anymore with the value True (should be that same as DEF, but practically it is not printing anything in the cml after -ha). This needs to be restored for simplicity and also it should be the default, since non-ha calculations are very inaccurate in Zeopp. I guess also ha: False is not working.

@danieleongari The ha behavior has been changed in this commit : https://github.com/ltalirz/aiida-zeopp/commit/1ed83fae563c531c0d77d52f4ea9e75435892c29 Now, it accepts all the possible options available in zeopp. It can be used as 'ha' : 'DEF' for instance or just be ignored for low accuracy calculations.

• I would remove the workflow blockpocket since it is not compatible with AiiDA-1 and it is anyway of little use: if one wants to include the calcualtion of the blocking spheres in e.g., a MC simulation, it is simpler to include it in that workchain instead of nesting workchains. In general I don't see the need of including workchains in the plugin of zeopp, being such a simple code.

I was thinking the same today and about just adding similar base workchain here to handle the common failures.

[ltalirz]

Thanks for the notice on the bug - I'd happily accept a pull request.

Even if the workchain may not seem the most useful one, I don't really see the need to delete it (we should add a test for it, of course). As for adding a base workchain, that seems like a good idea.

[danieleongari]

So from now on let's use "ha":"DEF" instead of "ha":True", no problem: in this case the plugin is not that popular, but next time please be more careful before breaking the API of the input, since it was not trivial at all to figure our in a bigger WC why it crashed and what was the solution!

I made a PR for updating the example/submit.py (and also to make it faster and more meaningful), where ha is used correctly.

Regarding the other issues, I believe that Zeopp is one of the simplest codes but best designed plugins, and therefore it would be nice to keep it nice and simple.

• I don't see the need of a base WC: what is it supposed to do? Can we avoid to over-complicate such a simple code?
• regarding the blockpocket workchain, I do not see the purpose of keeping it. It is deprecated (not fully compatible with the plugin and not tested), not logical (it computes the VOLPO to decide whether to compute the blocking spheres.... but VOLPO takes way more time than blockpockets!) and misleading (if someone does not use the -rad and probe size coherent to the FF used for the simulation it does not makes much sense, therefore I don't see how it can stay separate from a Raspa WC). We could keep it for educational purposes, but I think we have better WC for that.

[ezpzbz]

• I don't see the need of a base WC: what is it supposed to do? Can we avoid to over-complicate such a simple code?

The reason behind my thought about having BaseRestartWorkChain in aiida-zeopp plugin is that in our other multi-code complex workchains, the other plugins are safe (in most of the cases) from the system failures (like walltime or memory exceeding) while ZeoppCalculation currently cannot benefit from this. It would become more important when we have several ZeoppCalculation calls in a higher-level workchain and if any of them of them crashes for above-mentioned reason, the whole WC dies. Once we have have more demanding zeopp calculation at accuracy levels above DEF which can use huge amount of memory, it can become more important.

[danieleongari]

I see your point but calculations that require very high time or memory are or EXTRIMELY big structure (that would fail in whatever other program, so if they crash in Zeopp better, we can exclude them) or for experimental cases, like the ones you are running, but not for general use. Therefore, I would be reluctant to use base in bigger WC as a "standard" (like we do for Raspa and CP2K), but of course for your specific use it may be interesting.

[ltalirz]

So from now on let's use "ha":"DEF" instead of "ha":True", no problem: in this case the plugin is not that popular, but next time please be more careful before breaking the API of the input, since it was not trivial at all to figure our in a bigger WC why it crashed and what was the solution!

You are absolutely right, and I apologize for this. I will see to making the plugin backwards-compatible again - should not be a big deal.

I made a PR for updating the example/submit.py (and also to make it faster and more meaningful), where ha is used correctly.

Thanks!

regarding the blockpocket workchain, I do not see the purpose of keeping it. It is deprecated (not fully compatible with the plugin and not tested), not logical (it computes the VOLPO to decide whether to compute the blocking spheres.... but VOLPO takes way more time than blockpockets!) and misleading (if someone does not use the -rad and probe size coherent to the FF used for the simulation it does not makes much sense, therefore I don't see how it can stay separate from a Raspa WC).

Hm... ok. I think we can remove it for the moment.

We could keep it for educational purposes, but I think we have better WC for that.

An alternative educational workchain for zeo++ would be very welcome (if something like this makes sense).

Regarding a "base workchain": I think workchains that handle basic failure modes are welcome for every plugin. The description of the workchain class can make it clear when using this workchain can be useful (note: sebastiaan modified the verdi plugin list such that it should now also show the description of the class).

[ltalirz]

The workflow is removed. I'll now look into making the ha option backward compatible again. This should be the last issue before the next aiida-zeopp release.