Made bracketing of chemical potential more robust #43
Conversation
|
@EthanMakaresz can you fix up the ruff linting. Is it possible to add a unit test/regression test for a case where this is needed? |
| new_sign = np.sign(target_function(e_max)) | ||
|
|
||
| if old_sign == new_sign: | ||
| e_min, e_max = adjust_brackets(e_min, e_max) |
There was a problem hiding this comment.
I think this should be
return adjust_brackets(e_min, e_max)
There was a problem hiding this comment.
I think you're right here. I'll change this.
| """ | ||
| old_sign = np.sign(target_function(e_min)) | ||
| if old_sign > 0: | ||
| e_min -= 0.5 |
There was a problem hiding this comment.
Should the amount it steps be normalized in some way? What if the energy bracketing is very small? Or, have adjust_brackets take an optional step size.
There was a problem hiding this comment.
My thinking is that N(mu) should be a monotone increasing function, so a too-large step size here won't be an issue. But I can adjust it to step by (e_max - e_min)/4 or something like that.
There was a problem hiding this comment.
I did some testing with a step size delta = (e_max - e_min)/2, and it failed because delta was 10^-8. I've not run into any issues with using 0.5, and I know for example that this is what triqs solid_dmft does to bracket mu. Another option is to pass delta = max([(e_max - e_min)/2, 1e-3]). This worked for me, but the choice of 1e-3 is arbitrary.
|
I'm not sure how to do unit tests on github. I'll look into it and add one soon. |
I just mean a test like in tests/test_latticesolver.py |
|
Do you mean just add a test to test_latticesolver? It'll take a little while, I haven't used pytest before. |
2 participants
Often the brackets of the chemical potential wouldn't work and the calculation would fail, so now if that happens I just adjust the brackets until they work. Infinite recursion is avoided by Python's default recursion limit. I tested it on a problem that used to fail, and now it works seamlessly on my machine.