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: Add protocols for the hp.x work chains #14

Closed
wants to merge 1 commit into from

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented Apr 22, 2021

Fixes #15

Note: This PR is still very much a draft, but I can't create Draft PR's. 😭

Add the protocols for the hp.x workchains by:

  • Adding a get_builder_from_protocol() classmethod to the work chains.
  • Providing the corresponding .yaml file with the protocol.

To make this work, I needed to make a change in the aiida-quantumespresso plugin, see this PR.

Questions/TODO's:

  • Tests once we agree on implementation
  • Docstrings + help.
  • RelaxType: @sphuber since we pass the kwargs to the get_builder_from_protocol() method of the PwRelaxWorkChain, one could pass RelaxType.NONE to the get_builder_from_protocol() method of the SelfConsistentHubbardWorkChain and this will be passed to the PwRelaxWorkChain. However, won't this result in still running the PwRelaxWorkChain (but then as an 'scf') unnecessarily? In fact, don't we have a similar problem for the PwBandsWorkChain?
  • ElectronicType: Add logic to skip recon SCF in case this is provided.
  • Currently we do not have a kpoints_distance equivalent for the qpoints, so I just took a fixed qpoints mesh for the protocol. This is clearly not the correct approach. Question is where to add the qpoints_distance input, since it requires the structure.
  • Since starting from a more reasonable initial guess for the U value, I've added the hubbard_u.yaml file which so far just contains shaken-out-of-my-sleeve suggestions for Fe and Co. It's mainly to show the principle and implementation, we'll have to discuss with @timrov to develop a good approach for getting reasonable initial guesses.

@sphuber
Copy link
Contributor

sphuber commented Apr 23, 2021

shaken-out-of-my-sleeve

Haha 😆 great bit of Dunglish 👍

RelaxType: @sphuber since we pass the kwargs to the get_builder_from_protocol() method of the PwRelaxWorkChain, one could pass RelaxType.NONE to the get_builder_from_protocol() method of the SelfConsistentHubbardWorkChain and this will be passed to the PwRelaxWorkChain. However, won't this result in still running the PwRelaxWorkChain (but then as an 'scf') unnecessarily? In fact, don't we have a similar problem for the PwBandsWorkChain?

I take it the problem here is that if RelaxType.NONE the PwRelaxWorkChain is still run but in "scf" mode. We could simply check for this value in the get_builder_from_protocol in the higher-level workchain, and is set to RelaxType.NONE we simply pop the relax namespace from the builder. This should prevent the relax workchain from being launched. At least for the PwBandsWorkChain, not sure if the SelfConsistentHubbardWorkChain is coded the same way).

@mbercx
Copy link
Member Author

mbercx commented May 7, 2023

@bastonero I suppose this PR has been massively superseded by #32? Unless you still spot anything of value feel free to close it.

@bastonero bastonero closed this May 7, 2023
@mbercx mbercx deleted the feat/protocols branch May 7, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Protocols: Add get_builder_from_protocol method to work chains
3 participants