Fix incorrect comparison logic and update tests

[naik-aakash]

Closes #4166

Changes

Based on the suggestions form @QuantumChemist ( https://github.com/materialsproject/pymatgen/issues/4166#issuecomment-2480508677) and @DanielYang59 (https://github.com/materialsproject/pymatgen/issues/4166#issuecomment-2480925279) , the code has been fixed to correctly evaluate whether the matrix values are above threshold and tests have been updated accordingly

Todo

• [x] Add 1 or 2 more test cases

[JaGeo]

@naik-aakash could you add a few more tests here next week (not today) and make sure it is working as intended?

[naik-aakash]

@naik-aakash could you add a few more tests here next week (not today) and make sure it is working as intended?

Sure can do this 😄

[DanielYang59]

Forgive me, I'm going slightly off-topic again (Haoyu the topic wanderer), perhaps also replace the tolerance (limit_deviation) with more readable scientific notation? https://github.com/materialsproject/pymatgen/blob/c72c9e93ca5922f8fbd1d0e0bb385cfc1ae26420/tests/io/lobster/test_outputs.py#L1545-L1604

Also perhaps separate the tests of has_good_quality_maxDeviation and has_good_quality_check_occupied_bands into two separate test methods? (I guess they're independent right?)

[naik-aakash]

Forgive me, I'm going slightly off-topic again, perhaps also replace the tolerance (limit_deviation) with more readable scientific notation?

https://github.com/materialsproject/pymatgen/blob/c72c9e93ca5922f8fbd1d0e0bb385cfc1ae26420/tests/io/lobster/test_outputs.py#L1545-L1604

Also perhaps separate the tests of has_good_quality_maxDeviation and has_good_quality_check_occupied_bands into two separate test methods? (I guess they're independent right?)

How about renaming limit_deviation to identity_deviation_tol ? Does this make it more readable scientific notation ?

And regarding your second request, yes they are independent and thus have been seperated now in the tests.

[DanielYang59]

How about renaming limit_deviation to identitiy_deviation_tol ? Does this make it more readable scientific notation ?

Sorry I was trying to say replace values like 0.000001 with 1e-6, renaming limit_deviation might be breaking I'm afraid (has_good_quality_check_occupied_bands is public right)?

And regarding your second request, yes they are independent and thus have been seperated now in the tests.

Thanks for the quick change!

[naik-aakash]

has_good_quality_check_occupied_bands

Ahh okay got what you meant. Was not clear to me it referred to numeric values and not vairable naming and Yess it is public, thus will lead to a breaking change.

[shyuep]

Thanks.