Bugfix for bz_integration for edge case where mu and H(kx,ky,kz = 0) - #1023
Open
jcoulter12 wants to merge 2 commits into
Open
Bugfix for bz_integration for edge case where mu and H(kx,ky,kz = 0) #1023jcoulter12 wants to merge 2 commits into
jcoulter12 wants to merge 2 commits into
Conversation
…ide by zero. - In a case where the gamma point H(k) is zero, mu is zero, the explicit function evaluations in bz_integrators' deduce_dim_from_expression encounters a divide by zero. - Most straightforward fix is to pass one frequency point into the function to avoid using 0. Co-authored-by: David Abramovitch <dabramovitch@caltech.edu>
…havior. - Remove completely unused integrate_ptr function, which was outputing integrated quantities stored in a gf object. Was never instantiated, can be added back if needed later. - Minor update to printing during verbose mode, which caused incorrect printing of the remaining number of points. - Allow calculation to run the max specified kgrid instead of running only up to that grid size. Co-authored-by: David Abramovitch <dabramovitch@caltech.edu>
Contributor
Author
|
I realized there may be one more even narrower edge case to handle, maybe delay review until I've looked at it. |
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
1 participant
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.
In an edge case where both mu and H(k) at specifically the kx,ky,kz = 0 point, gloc or find_chemical_potential fail as nda::linalg::inv_in_place error. This originated from the deduce_dim_from_expression function, which was filling in placeholders with kx,ky,kz = 0, w=0 to evaluate the lazy expression to be integrated and find it's dimension.
This has been fixed by using the first mesh point on which a function will be evaluated instead of w=0.
Additional minor changes:
-- An unused integrate_ptr function was removed. It was never instantiated and had implementation issues anyway -- it was designed it seems, to output integrated k quantities to a gf container. We can add this later if it seems it's needed.
-- There was a problem in verbose mode with printing the remaining number of unconverged frequency points.
-- Upon suggestion from @dabramovitch, we changed the meaning of k_grid_max. Previously, we ran up to but not including the max grid. This would change the behavior to be more intuitive, running also the maximum specified grid density. As in, if k_grid_max = [10,10,10], and k_grid incremented in units of 1, before the last k_grid run would be 9,9,9. Now it will also run the 10,10,10 one before stopping.
Thanks to @dabramovitch for reporting and helping to locate this issue.