Important bugfixes to QHACalc now that fixed-cell relaxations are carried out#159
Merged
rul048 merged 33 commits intomaterialyzeai:mainfrom Mar 8, 2026
Merged
Important bugfixes to QHACalc now that fixed-cell relaxations are carried out#159rul048 merged 33 commits intomaterialyzeai:mainfrom
QHACalc now that fixed-cell relaxations are carried out#159rul048 merged 33 commits intomaterialyzeai:mainfrom
Conversation
Signed-off-by: Andrew S. Rosen <asrosen93@gmail.com>
This is a follow-up to materialyzeai#158 to ensure that the `fmax` and `optimizer` are consistent between the different relaxations in the `QHACalc`. Signed-off-by: Andrew S. Rosen <asrosen93@gmail.com>
Signed-off-by: Andrew S. Rosen <asrosen93@gmail.com>
Signed-off-by: Andrew S. Rosen <asrosen93@gmail.com>
QHACalcQHACalc and improve fmax default
Contributor
Author
|
This is now ready for review. Tests pass locally, but the CI is messed up. |
QHACalc and improve fmax defaultQHACalc
Signed-off-by: Andrew S. Rosen <asrosen93@gmail.com>
4 tasks
QHACalcQHACalc now that fixed-cell relaxations are carried out
QHACalc now that fixed-cell relaxations are carried outQHACalc now that fixed-cell relaxations are carried out
Contributor
|
I'll take a closer look and try to fix the CI issues as soon as possible. |
Andrew-S-Rosen
commented
Mar 7, 2026
Andrew-S-Rosen
commented
Mar 7, 2026
Andrew-S-Rosen
commented
Mar 7, 2026
Signed-off-by: Andrew S. Rosen <asrosen93@gmail.com>
4 tasks
Contributor
|
Thanks for the improvements and for addressing these issues with |
4 tasks
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.
This PR fixes additional bugs with the
QHACalc, following up #158, in which I fixed a bug where the phonon calculations were being carried out on the scaled structures without any ionic relaxation carried out. It also closes #138 by adding support for checking imaginary modes.Using the correct energies
The energies used for the EOS fit should be taken from the minimum energy structures at the fixed-volume relaxations. This was not done before but is now done.
Consistent settings
The
fmaxandoptimizershould be used for all relaxations consistently. This was not done before but is now done by passing the arguments to allRelaxCalccalls.Relax cell shape
While a fixed-cell volume relaxation should be carried out, the cell shape should be allowed to change. I have enabled this. In doing so,
RelaxCalctakes a new keyword argumentrelax_cell_kwargsso that we can doFrechetCellFilter(constant_volume=True).Imaginary modes
There is now a keyword argument
imaginary_freq_tolto optionally check for imaginary frequencies.Force tolerance
I also have better addressed #127. In short, the existing
fmaxdefault value is not appropriate for phonon calculations. The forces must be very small to carry out accurate phonon calculations. The standard in the literature varies anywhere from 1E-5 to 1E-8 eV/A. The default of 0.05 eV/A (while lower than 0.1 eV/A before) is not scientifically sound. People will be doing very bad science with this kind of default. For an example, in this MLIP paper, the forces are converged to 1e-8 eV/A. I have madefmaxset to 1e-5 as a better default. I also increasedmax_stepsto 5000 to ensure the calculations are reach convergence.