PwRelaxWorkChain: Revisit work chain logic
#952
Conversation
|
@sphuber some initial notes/questions/decisions:
|
There was a problem hiding this comment.
Thanks @mbercx . I think this is looking good. Left some comments.
Should the initial relaxation be run by default? Currently it is, and if we don't make it the default I'm wondering how to allow the user to enable it while also being able to provide a protocol for this step.
Is it the default? If the base_init_relax namespace is not defined at all, it won't be run. Do you maybe mean the default when using get_builder_from_protocol? If it works really well for most real use-cases, I think it makes sense to actually use it by default. We can add a note in the documentation how to skip it, which should not be complicated.
Re the documenation: I was planning to describe the work chain Purpose and Logic in the Topics section. What do you think?
Yeah, makes perfect sense.
| base_init_relax: | ||
| kpoints_distance: 0.3 | ||
| meta_parameters: | ||
| conv_thr_per_atom: 1.e-6 | ||
| etot_conv_thr_per_atom: 1.e-3 | ||
| pw: | ||
| parameters: | ||
| CONTROL: | ||
| calculation: scf | ||
| forc_conv_thr: 0.5e-2 | ||
| CELL: | ||
| press_conv_thr: 2 |
There was a problem hiding this comment.
Is there an origin for these numbers other than your thumb? :D Not that I have any better ones per se, but just curious if these are already tested in someway
| } | ||
| } | ||
| new_kpts_mesh, _ = create_kpoints_from_distance(**inputs_create_kpoints).get_kpoints_mesh() | ||
| if tuple(input_kpts_mesh) != tuple(new_kpts_mesh): |
There was a problem hiding this comment.
EDIT: just saw your point in the PR description. Will leave this here because it contains some more thoughts on the exact criterion.
Should we maybe here check only that the new grid is not denser? If the new required grid is less dense, then there is no point in rerunning is there? The criterion for kpoints_distance that a minimum density is guaranteed in order to guarantee a minimum precision.
One tricky thing I guess might be if you have non isometric meshes, such as (2, 3, 1) and (3, 2, 1). Which one is denser? I guess there you would have to compute the actual distance between kpoints in 1/A for both both meshes in each direction and as soon as just one is larger for the new structure, we conclude we have to rerun.
There was a problem hiding this comment.
I'm not sure how common that case last case is, but it's a fair point. Maybe we can accept when the new mesh is denser along every direction. This would not accept your last use case, but I think that's fine, and simpler than really calculating the density.
mbercx
force-pushed
the
fix/705/revisit-relax
branch
from
fae0e0d
to
3ed7e91
Compare
mbercx
force-pushed
the
fix/705/revisit-relax
branch
from
3ed7e91
to
aa6eb9d
Compare
mbercx
force-pushed
the
fix/705/revisit-relax
branch
from
de02473
to
b21fc70
Compare
mbercx
force-pushed
the
fix/705/revisit-relax
branch
from
b21fc70
to
1dfdade
Compare
Fixes #733
Fixes #705
Will write full commit message when all changes are agreed upon.