from_scratch for bands calculation

[unkcpz]

fixes #877

[sphuber]

Thanks @unkcpz . I agree that this change is correct (QE docs say not to use restart for non-scf calculations https://www.quantum-espresso.org/Doc/INPUT_PW.html#idm65). Just please remove the print statement

[sphuber]

Suggested change

print(parameters)

[unkcpz]

Ah, okay. Please ignore my comment. I write it at the same time as your comment.

[unkcpz]

Removed.
I also make a mistake by pushing directly to main. Hopefully, (and please pretend) you didn't notice @sphuber 😿

[sphuber]

[unkcpz]

I print here and it shows the parameter is set to "restart", I don't know where this is set from.

[sphuber]

Is this still true? Or is the value now actually correctly getting set to from_scratch? I am confused whether this is now working as intended and fixes the issue

[unkcpz]

Yes, it is now working. I test locally and the warning disappeared. More importantly, the restart_mode of band calcualtion is from_scratch if check from the remote folder.

[mbercx]

I remain somewhat opposed to the approach of setting/overriding inputs inside the steps of the work chain in case it can be avoided. A couple of thoughts:

1. Overriding inputs inside the work chain can lead to confusion in case the user has explicitly set a different value, and will prohibit the user from changing it unless logic is implemented to respect explicitly provided inputs. In some cases it's true that it seems you have to set a certain value (e.g. from_scratch for a bands calculation), but perhaps we don't know the use case (this was also the reason for removing the line that sets the restart_mode in PwBandsWorkChain: do not define restart_mode in bands step #475).
2. If you set/override inputs inside the work chain, it's more difficult for the user to find where this value comes from when he/she is checking the input files. We already have a place for setting the inputs of a work chain: the protocol files. So I think it makes sense to explicitly set default values there. This also respects the original "separation of church and state" principle which lead to the concept of defining a protocol.

@sphuber @unkcpz if you agree, I can work on a PR that implements these concepts more broadly for the PwBandsWorkChain.

[unkcpz]

Thanks @mbercx, please go ahead. I agree the interface needs to be more unified and better to be only in one place for all override changes.

[mbercx]

Another reason to put these inputs in the protocol instead: the builder repr now looks like this:

Process class: PwBandsWorkChain
Inputs:
bands:
  metadata: {}
  pw:
    code: Plane wave Quantum ESPRESSO v7.2 - local compilation
    metadata:
      options:
        max_wallclock_seconds: 43200
        resources:
          num_machines: 1
        stash: {}
        withmpi: true
    monitors: {}
    parameters:
      CONTROL:
        calculation: scf
<...>

So it looks like we'd actually be running an scf for the bands step, but this is updated inside the work chain...

[mbercx]

See #902

[mbercx]

Superseded by #902