Implement common bands work chain for Quantum ESPRESSO

[mbercx]

Implementation of the common bands work chain for Quantum ESPRESSO. A couple of notes:

• Currently the number of bands is not changed compared to the parent calculation. As discussed in Bands common workflow #222, we might want to introduce a way to specify the number of bands. Using an energy range is tricky, since we do not know a priori how many bands are required to obtain a specified energy range. For the PwBandsWorkChain in the aiida-quantumespresso plugin, we have an nbands_factor input, which simply increases the number of bands based on the default from the SCF calculation.
• Also adds the remote_folder output to the QuantumEspressoCommonRelaxWorkChain so this can be passed to the input generator of the QuantumEspressoCommonBandsWorkChain. However, there is another issue here, related to the fact that by default the protocol will set clean_workdir to True for the PwRelaxWorkChain, which means the folders will be cleaned. I now added another input to the generator for the QE relax work chain, but I'm not sure if this is the best approach since it changes the interface? Probably having access to the inputs via overrides might be preferable.

[sphuber]

Thanks @mbercx . Gave a first pass. I see the problem of the clean_workdir, but not yet sure if we should add this to the generator. It feels like there may be a better solution, but have to think about it some more.

[sphuber]

few more instances of builder_common_bands_wc lying around. Would be good if you could try to run it at least once in absence of unit tests.

[mbercx]

Huh, I think this is only in e81a8e2, in the latest commit (2e298cb) this should be fixed. It seems I was a little careless in accepting the reorganisation of the builder code however, I fixed this in c1d0695. Now I tested the relaxation + bands work chain for Si, and it seems to be running fine again.

[sphuber]

I must have seen it after that first commit. Then I haven't said anything :D

[bosonie]

@mbercx thanks for the implementation. It looks great.
The only discussion we need to have is regarding the additional input clean_workdir you introduced for the QuantumEspressoCommonRelaxInputGenerator.
I think that I am coming to the realization that allowing code implementations to add inputs to the CommonRelaxInputGenerator must be discouraged. In fact, the code-dependent inputs become unusable in higher level code-agnostic workchains.
Imagine to have now a code-agnostic workchain that makes the relaxation and subsequently the calculation of bands. The code agnostic workchain will expose the "common" inputs of the relaxation but not of the additional quantum-espresso-only input. By definition the code-agnostic workchain will be aware of the implementation to use only at runtime, making it impossible to deal with code specific features before. Of course one could allow generic extra inputs and then put a check inside the workchain saying: "if the selected implementation is QE, then check for this input", but I feel this is very difficult to maintain since should be implemented for every new code-dependent input and for every higher level workchain. Even more because we are now working to have a robust implementation of "overrides", meaning the possibility to allow code dependent modifications in a systematic way (see branch feature/relax_wc here).
Isn't it an option to make default clean_workdir = False in the QuantumEspressoCommonRelaxInputGenerator? Does QE generates so much data?
In any case we can talk tomorrow

[mbercx]

Thanks for the comments, @bosonie!

I think that I am coming to the realization that allowing code implementations to add inputs to the CommonRelaxInputGenerator must be discouraged. In fact, the code-dependent inputs become unusable in higher level code-agnostic workchains.

I see your point, and I agree. I also think adding engine-specific code in the higher-level work chains is not the best approach because ideally we would be able to make anything but the "base" common work chains completely code-agnostic. I'm not sure if we'll be able to achieve this though. Already the issue on how to define the number of unoccupied bands might be tricky to define in one inputs for the CommonBandsWorkChain. Some codes indicated that they would like to define this as an energy range vs the Fermi level, but this approach is not very suitable for Quantum ESPRESSO.

Even more because we are now working to have a robust implementation of "overrides", meaning the possibility to allow code dependent modifications in a systematic way (see branch feature/relax_wc here).

Overrides will definitely help, but then either the user or the higher-level work chain needs to be aware of having to set clean_workdir to False when running the RelaxAndBandsWorkChain with Quantum ESPRESSO. Expecting the user to override this properly is not a very friendly approach ^^. One way might be to allow the developer of each engine to define protocol overrides for higher-level work chains.

Isn't it an option to make default clean_workdir = False in the QuantumEspressoCommonRelaxInputGenerator? Does QE generates so much data?

I'd vote against this because of my experience with running a lot of calculations in high-throughput. If you forget to set clean_workdir to True, you will quickly run into the file limit on most scratch quotas. Quantum ESPRESSO likes to make a lot of files, especially when running with a dense k-point grid.

However, perhaps it is not unreasonable to make clean_workdir an input for all common work chains? Is seems like a setting that's pretty generic.

[bosonie]

@mbercx you are actually right. Not bad to make clean_wordir an input for everybody. I will suggest it today. But I will be in favour to make it False as default