👌 Only require pw.x in get_builder_from_protocol when it's actually needed - #101
Open
elinscott wants to merge 3 commits into
Open
👌 Only require pw.x in get_builder_from_protocol when it's actually needed#101elinscott wants to merge 3 commits into
pw.x in get_builder_from_protocol when it's actually needed#101elinscott wants to merge 3 commits into
Conversation
`Wannier90WorkChain.get_builder_from_protocol` always assembled the scf and nscf namespaces and required a `pw` code, so a caller that reuses a charge density or an nscf folder computed elsewhere had to pass a pw.x code that never runs and then delete those namespaces from the builder. - Add an `nscf_parent_folder` argument: neither pw namespace is assembled and the folder becomes the pw2wannier90 `parent_folder`. - Add an `scf_parent_folder` argument: the scf namespace is skipped and the folder becomes the nscf `parent_folder`. - Reject passing both, naming which one starts from which step. - Require the `pw` code only when a pw namespace is assembled. - Give `check_codes` a `required_codes` argument, matching the existing parameter of the same name in `utils/code.py`. - Read the semicore pseudopotentials from whichever pw namespace is present, and raise when `exclude_semicore` is requested with neither. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`nscf_parent_folder` refused `exclude_semicore=True`, because the semicore states were read off a pw namespace and neither is assembled on that route. The pseudopotentials the states come from are fixed by the pseudo family and the structure, both already resolved by then. - Take the pseudos from `get_pseudo_and_cutoff` when no pw namespace was assembled, and drop the refusal. - Cover it with an equality against the full-assembly path on structures that have semicore states. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pw.x in get_builder_from_protocol when it's actually needed
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
==========================================
- Coverage 43.25% 37.68% -5.57%
==========================================
Files 44 45 +1
Lines 4231 5172 +941
==========================================
+ Hits 1830 1949 +119
- Misses 2401 3223 +822 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…-scf-nscf-builder
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Wannier90WorkChain.get_builder_from_protocolalways assembles thescfandnscfnamespaces and always demands apwcode, even though the workchain itself might not require them (it runs those steps only when the namespace is present in its inputs).A caller reusing a charge density or an nscf folder computed elsewhere is forced to provide a pw.x code that never runs, then delete both namespaces from the returned builder before submitting.
Changes
nscf_parent_folderargument toWannier90WorkChain.get_builder_from_protocol: if present, neither pw namespace is assembled, and the folder becomes the pw2wannier90parent_folder, so the builder starts at the wannier90 step.scf_parent_folderargument: if present, the scf namespace is skipped, the nscf is still assembled, and the folder becomes the nscfparent_folder.pwcode only when a pw namespace is assembled, sonscf_parent_folderlets the caller omit it fromcodesentirely.check_codesinutils/workflows/builder/submit.pyarequired_codesargument, matching the parameter of the same name already inutils/code.py.exlcude_semicoreworking, read the semicore pseudopotentials from whichever pw namespace is present, and from the pseudo family and the structure when neither isThe builder comes back ready to submit rather than needing to be patched up afterwards. Defaults are unchanged.
Testing
nscf_parent_folderand nopwcode passesWannier90WorkChain.spec().inputs.validate()scf_parent_folderscf_parent_folderstill requirespwnscf_parent_folder,exclude_semicoreproduces exactly the semicore list the full-assembly path producespwcode with the original error