search

siesta-project / aiida_siesta_plugin

Source code for the AiiDA-Siesta package (plugin and workflows). See wiki
Other
6 stars 11 forks source link

Fix bands path #80

Closed bosonie closed 3 years ago

bosonie commented 3 years ago

This PR introduces various changes: 1) Fixes #79, in the sense that the parser now will use the final structure to set the distance between kpoints (for plotting purposes) if a relaxation has been performed. Kpoints are in fact always passed in reciprocal axis units. 2) Fixes #79, in the sense that we now allow to pass to banskpoints a set of kpoints in reciprocal space without the need of specifying a cell. 3) Imposes that, if a cell is set in bandskpoints, it must be the same one of the input structure. 4) Introduces a warning if a relaxation with variable cell is requested together with bands. 5) Tries to make the BangGapWorkChain smarter introducing a new feature: if no bandskpoints is specified in input, the workchain sets the calculation of bands automatically. If single-point calculation, the bands are set using seekpath since the beginning. If a relaxation was requested, the relaxation is performed and the bands are calculated on an extra final step, where seekpath sets the kpoints path for bands using the final structure. Please remember that seekpath might change the structure to follow conventions. Also note that the choice is fully back compatible. If a users specify bandskpoints in input, the same behavior of before is set, including the limitation that the kpoints are set in the beginning and might result to be insignificant if the cell changes. However this is also possible in Siesta itself.

@pfebrer96's review is requested to check if the implementation fits his needs and it is correct (if would be appreciate a comparison between bands parsed here and maybe sisl. @albgar's review would be appreciated for point 5).

bosonie commented 3 years ago

I felt that a BandGapWorkChain with no calculation of bands was useless, therefore the initial idea would have been to set anyway "automatic" as a default. In this way, the API is the same, and I have more flexibility in the future to decide what to do, I'm not sure that seekpath will stay the automatic generator forever.

albgar commented 3 years ago

What happens if there is relaxation and the final structure has a lower symmetry because of noise? Would seekpath be confused and lead to a BZ path that is not what one expects, or does it have a tolerance for 'approximate symmetry' and would recognize it? Also, in what sense might the k-points specified in an input bandskpoints become inappropriate? Aren't they (or shouldn't they) be specified in terms of the reciprocal lattice vectors, and adjusted so that the "k-distance" among them fits in the line(s)?

bosonie commented 3 years ago

What happens if there is relaxation and the final structure has a lower symmetry because of noise? Would seekpath be confused and lead to a BZ path that is not what one expects, or does it have a tolerance for 'approximate symmetry' and would recognize it?

The method seekpath.get_explicit_k_path accepts symprec and angle_tolerance inputs to set the precision at which to recognize the symmetry of a structure. I was working this morning to give the option to users to change these parameters.

Also, in what sense might the k-points specified in an input bandskpoints become inappropriate? Aren't they (or shouldn't they) be specified in terms of the reciprocal lattice vectors, and adjusted so that the "k-distance" among them fits in the line(s)?

They are specified in reciprocal lattice vectors units, therefore if only the volume changes, no problem. However it might happen that there is a phase transition during md (we do not have any constrain on what a relaxation is). In this case, the bands path must be selected from the output structure. The machinery implemented here wants to cover this case.