JaGeo / LobsterPy

Package to perform automatic bonding analysis with the program Lobster in the field of computational materials science and quantum chemistry
https://jageo.github.io/LobsterPy/
BSD 3-Clause "New" or "Revised" License
74 stars 27 forks source link

[CI] `test_cli` failed on MacOS due to different `matplotlib` rounding precision #297

Closed DanielYang59 closed 2 months ago

DanielYang59 commented 2 months ago

This last line: https://github.com/JaGeo/LobsterPy/blob/2ec4d19edab95a18c36817b5ab764de73d7330de/tests/cli/test_cli.py#L92

of https://github.com/JaGeo/LobsterPy/blob/2ec4d19edab95a18c36817b5ab764de73d7330de/tests/cli/test_cli.py#L72-L92

Would fail only on MacOS (passed on Ubuntu):

>                   assert plot_attributes[key] == pytest.approx(ref_value)
E                   assert [20.0, 12.36] == approx([20.0 ...96 ± 1.2e-05])
E                     
E                     comparison failed. Mismatched elements: 1 / 2:
E                     Max absolute difference: 0.0006797749978968426
E                     Max relative difference: 5.499797717611995e-05
E                     Index | Obtained | Expected                    
E                     1     | 12.36    | 12.360679774997896 ± 1.2e-05
JaGeo commented 2 months ago

@DanielYang59 You are invited to make a fix. We don't test for MAC at the moment. We can also add additional tests for mac if you think this is useful.

DanielYang59 commented 2 months ago

With pleasure :) I need a closer look at the reason though, because I would expect MacOS to be very similar to Ubuntu. Would do this very soon.

JaGeo commented 2 months ago

Thanks! We simply only use Linux and therefore never checked the mac things!

DanielYang59 commented 2 months ago

I just had another look at this issue, and it seems to come from the difference in rounding precision from matplotlib.

The following test case: https://github.com/JaGeo/LobsterPy/blob/55d8d2e119aa1147166994d57fbdbe10931cc748/tests/cli/test_cli.py#L38

The generated figure height is 12.360679774997896, completely agreeing with the reference on Ubuntu.

While on MacOS, only two decimal places are kept as 12.36, which leads to this test failure as the default relative tolerance is 1E-6.

To recreate this:

import matplotlib.pyplot as plt
import matplotlib as mpl
from math import sqrt

width = 20
ratio = (sqrt(5) + 1) / 2  # the golden ratio in _user_figsize func

height = width / ratio
mpl.style.use({"figure.figsize": (height * ratio, height)})

print(mpl.rcParams['figure.figsize'])  # >>> [20.0, 12.360679774997896]

print(plt.gcf().get_size_inches().tolist())  # >>> [20.0, 12.36] on MacOS / [20.0, 12.360679774997896] on Ubuntu

However I don't know exactly what caused the figsize to be rounded differently, might need to look into the matplotlib source code.