Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocols: Extend protocol concept to CalcJob classes #947

Open
mbercx opened this issue May 27, 2023 · 0 comments
Open

Protocols: Extend protocol concept to CalcJob classes #947

mbercx opened this issue May 27, 2023 · 0 comments

Comments

@mbercx
Copy link
Member

mbercx commented May 27, 2023

Currently, only WorkChain classes use the ProtocolMixin class utilities and define a get_builder_from_protocol() method. I see no reason why CalcJob classes couldn't also benefit from such a method. There are several advantages to this:

  1. Some CalcJob classes don't have a corresponding BaseRestartWorkChain, but could still benefit from having a default protocol defined.
  2. In some cases a user might not want to use the restart features, but still use a protocol.
  3. In the tutorials, it would be natural to start with explaining how to run a PwCalculation using the get_builder_from_protocol() method, but now this isn't possible. Instead, we immediately have to explain how to use the PwBaseWorkChain, which adds an extra layer of complexity.
  4. It would be easier to implement Protocols: Consider way of "popping" when overrides is None #653, since this would definitely be the "lowest" level of the get_builder_from_protocol() hierarchy, and hence we could simply pop keys in e.g. the parameters that have value None at this point.

There are probably still more, but at this point I'm pretty convinced this would be a sensible approach. What do you think, @sphuber?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant