Simplify get_builder_from_protocol in ProjwfcBandsWorkChain

[t-reents]

Hi @qiaojunfeng, thanks for this nice work!

We (well, mainly @AndresOrtegaGuerrero) are working on integrating the ProjwfcBandsWorkChain into AiiDAlab. Andres realized that it's not possible to provide Python datatypes in the overrides when using the get_builder_from_protocol method (I could confirm this behavior). One example is passing a float for kpoints_distance. However, this is possible in the AiiDA-QE plugin.

The problem occurs in the following part:

aiida-wannier90-workflows/src/aiida_wannier90_workflows/workflows/projwfcbands.py

Line 157 in d6794cf

inputs = recursive_merge_container(inputs, protocol_inputs)

The PwBandsWorkChain.get_builder_from_protocol transformed the relevant inputs into AiiDA datatypes, but the aforementioned call of recursive_merge_container merges the plain Python datatypes from the protcol_inputs again.

I fixed this behavior in this PR and generally tried to simplify the get_builder_from_protocol. As the ProjwfcBandsWorkChain is a child class of the PwBandsWorkChain, the majority of the preparation is already done in PwBandsWorkChain.get_builder_from_protocol.
For example, I'm not sure if the following is really needed:

aiida-wannier90-workflows/src/aiida_wannier90_workflows/workflows/projwfcbands.py

Lines 122 to 124 in d6794cf

	protocol_inputs = cls.get_protocol_inputs(
	protocol=protocol, overrides=overrides
	)

The protocol inputs are anyway passed to the get_builder_from_protocol methods of the sub-workchains and the ProjwfcBandsWorkChain doesn't have any additional inputs that are specific to this WorkChain.

I tested the changes and the results seem to be identical (at least within my tests). In case I removed parts that are necessary for the intended functionality because I misunderstood them, please let me know. I'm happy to revert certain changes/to adapt them.

[aiida-cla-bot]

All contributors have accepted the CLA ✅

---

_{You might need to click the "Update/Rebase branch" button to update the pull request and rerun the GitHub actions to pass the CLA check.
Posted by the CLA Assistant Lite bot.}

[t-reents]

I have read the CLA Document and I hereby accept the CLA

[AndresOrtegaGuerrero]

@t-reents could you update the branch and also solve the issue with the CLA Assistant error ? , I already tested the branch and it works.

[AndresOrtegaGuerrero]

LGTM! thank you for your job @t-reents !, I tested the PR no issues , @superstar54

[superstar54]

Hi @t-reents , thanks for the PR.

These lines should remain. They are used to read the protocol parameters from the file, projwfcbands.yaml. For the moment, the file only specifies clean_workdir. I think that's why you get identical results even after you remove them. However, more parameters may be added in the future.

[t-reents]

Hi @superstar54

I see the point that one might create a specific protocol for this WorkChain. I also mentioned this in the PR description. My point was that this WorkChain doesn't implement any additional inputs and basically inherits the PwBandsWorkChain only. I assumed that one would simply provide the individual changes via the overrides.

But yeah, it makes sense to have the possibility to specify a protocol to overwrite the defaults of the parents. Otherwise, one would always need to provide adjusted protocols for PwBandsWorkChain and ProjwfcBaseWorkChain.

[t-reents]

@superstar54 Coming back to the point, that the ProjwfcBandsWorkChain doesn't have additional inputs, I'd suggest to load the protocol inputs and merge the relevant parts with the overrides for the subsequent calls of PwBandsWorkChain and ProjwfcBaseWorkChain. In this way, we still have the possibility to provide custom protocols but also fix the initial problem that the transformed values get overwritten by the Python base types.

[superstar54]

Fine to me. If you do this, please also remove the projwfcbands.yaml file, and then add a detailed comment in the code that the WorkChain uses the parent protocol setting.

It would be good that @qiaojunfeng could have a quick look at it.

[AndresOrtegaGuerrero]

@superstar54 Do you know if there is any update about this PR ?

[t-reents]

I actually picked this up again this morning. @superstar54 By using the protocol_inputs as the overrides in the subsequent get_builder_from_protocols calls, we now also allow additional inputs in the protocol file.

[t-reents]

As a general comment, we realized that the issue would also be solved by upgrading to aiida-core==2.6.2 (@AndresOrtegaGuerrero encountered it in aiida-core==2.5.1). There it is possible to pass the Python base types to the builder. However, it is not clear when aiidalab-qe will be upgraded to that aiida-core version, and therefore, the changes in this PR are still useful.

[qiaojunfeng]

Sorry for the delay, all good, thanks!