EquationOfStateWorkChain: use builder._merge instead of builder._update to update with inputs from sub_process namespace

[sphuber]

The builder._update will not recursively merge the dictionaries which is not the expected behavior since the sub_process namespace is intended to allow to define overrides which should target specific inputs to set to a custom value. If that value is in a nested namespace, it should not replace the entire namespace, but just merge the overrides in. This PR on aiida-core implements the ProcessBuilderNamespace._merge method which will do exactly this. It will be shipped with v2.0 at which point we can use this here. Until then, the current behavior of the sub_process input namespace is bugged, and if we want to fix it now, we have to do the recursion ourselves, until we drop support for aiida-core==1.*. Or if this is a pain, someone could backport the _merge feature on aiida-core==1.6.*.

[giovannipizzi]

Indeed I was asking the same in the PR and then came here to open this issue, great that you already did :-)
As I mentioned, probably 1.6.x wouldn't be the right place - indeed, maybe copying the implementation here (and leaving the issue to replace it with _merge in 2.0) is the best approach?

[giovannipizzi]

As mentioned here, I realised while discussing today with @bosonie that even _merge might not be enough.
The usecase can become even more general. E.g. what if I want only to update one field inside the Settings or Parameters Dict? What if I want to append to a list in it? What if I want to remove a field in a Dict?

I guess having the update and merge functionality is good anyway, and maybe it could be a good default that covers most simple use cases.
But we might want to have more flexibility for the common workflows.
Now, the issue does not come with the common workflows itself (we fixed by having a generator, and letting users adapt the inputs). But with single workflows wrapping a common workflow, like the EOS.

Here the added complexity is that the EOS inputs are actual inputs to AiiDA - so this prevents e.g. defining a sub_process_callback, i.e. a function that would be called for each subprocess, get the inputs from the generator, and be allowed to changed them at wish.
Maybe, however, a general approach would be to define a sub_process_calcfunction_callback_name (ugly name, but just to explain what it is). You pass a valid calcfunction name, and when the EOS gets to the callback point, it will load the calcfunction, pass the inputs, and let it adapt them. We might also decide a calcfunction is just too much and use a simple function - to be discussed.

This might be a bit complex - so having simple functionalities like updating, merging (and maybe thinking if there are more use cases) would be goo, but also having this as a last resort I think would be very useful.

Finally, to support all functionalities, maybe we should define multiple namespaces (sub_process_update, sub_process_merge)? Or define a slightly more complex syntax to tell what you want to replace, what you want to merge, ...?

[giovannipizzi]

In any case, in preparation for the common workflows meeting, it might be good to at least have the _merge option available, so people can use it and we see (while running) which other use cases pop up (and we prioritise solving those)

[sphuber]

Note that I have backported the fix and released it in v1.6.5 so that should already work for now.

[giovannipizzi]

I'd like to make this issue more general, about (in general) how overrides should work.

Here are my thoughts (if we all agree, we might want to change the title of this issue, or otherwise open a brand new issue and move this comment there):

• first, a note: overrides are not really useful for the standard relaxation workflows (you can just act on the inputs from the input generator, before submitting), but are needed for the "actual common" workflows on top of them (EOS etc.). Example: I want to change some command line option for parallelisation, double the cutoff, ...

• I'm realising that "what to do" with the overrides will always be very code specific. It'll be hard to find a general "override" set of rules (examples: replace completely the node; merge dicts; but also more general things like take data from more than one node, do some logic and merge the results replacing some of the inputs; ...).

Therefore, my suggestion is that the best is to make the override input accepting a function/lambda instead.
The way it would work: if the 'override' input gets passed a function, this function is applied to the output of the input generator. The lambda function signature is to get the all inputs from the generator (or directly the builder? we can decide), act on them, and return updated inputs (or an updated builder). So each implementation can do what they want, even complex logic and we don't limit it.

(Note: We can still offer a few simple things like replacing and merging, if we feel it simplifies some common use cases. But maybe it's not needed?)

Providing "lambda override functions" that are code specific can also be interesting because an implementation can provide a "library" of them, for various purposes and with appropriate documentation, and these can just be reused and combined by the user, and passed to common workflows like the EOS.

I'm happy to hear comments about this approach.