Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated the allclose function to have less precision according to the dtype of input DNDarrays. #1177

Closed
43 changes: 39 additions & 4 deletions heat/core/logical.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ def local_all(t, *args, **kwargs):


def allclose(
x: DNDarray, y: DNDarray, rtol: float = 1e-05, atol: float = 1e-08, equal_nan: bool = False
x: DNDarray,
y: DNDarray,
rtol: Optional[float] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be discussed (e.g. in the PR talk): should we keep the NumPy/PyTorch-standard and choose some float limits as default (but adapted to Heats float32-standard datatype) or do we want to proceed differently and let tolerances by default be chosen automatically depending on the input datatypes?

atol: Optional[float] = None,
equal_nan: bool = False,
) -> bool:
"""
Test whether two tensors are element-wise equal within a tolerance. Returns ``True`` if ``|x-y|<=atol+rtol*|y|``
Expand Down Expand Up @@ -139,17 +143,48 @@ def allclose(
"""
t1, t2 = __sanitize_close_input(x, y)

# Here the adjustment of rtol and atol for float32 arrays increases the values of rtol and atol
# effectively decreasing the precision required to return True.
# By adjusting the tolerance values, it allows for a looser comparison
# that considers the reduced precision of float32 arrays.

try:
dtype_precision = torch.finfo(t1.larray.dtype).bits
except TypeError:
dtype_precision = torch.iinfo(t1.larray.dtype).bits

adjustment_factor = 1.0

if dtype_precision != 64:
adjustment_factor = 64 / dtype_precision

if rtol is None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, the default atol here is 1e-8 for float64 (as in NumPy). Surprisingly the same default atol is chosen in PyTorch as well, where the default datatype is float32, so I am not even sure whether maybe even the PyTorch default is a bit inconsistent. Maybe this needs to be discussed together with the issue mentioned above.

Nevertheless, I think that adjustment_factor needs to be chosen in another way: as a rule of thumb we know that machine precision for float64 is roughly of size 1e-16 and 1e-8 for float32; therefore, I think that just doubling the tolerance when switching from float64 to float32 will not be sufficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure...sir, I agree. I think adjustment_factor can be changed depending on the choice of some default values we consider. Please tell me after you have discussed this and come to a conclusion.

rtol = 1e-05
adjusted_rtol = rtol * adjustment_factor

else:
adjusted_rtol = rtol

if atol is None:
atol = 1e-08
adjusted_atol = atol * adjustment_factor

else:
adjusted_atol = atol

# no sanitation for shapes of x and y needed, torch.allclose raises relevant errors
try:
_local_allclose = torch.tensor(torch.allclose(t1.larray, t2.larray, rtol, atol, equal_nan))
_local_allclose = torch.tensor(
torch.allclose(t1.larray, t2.larray, adjusted_rtol, adjusted_atol, equal_nan)
)
except RuntimeError:
promoted_dtype = torch.promote_types(t1.larray.dtype, t2.larray.dtype)
_local_allclose = torch.tensor(
torch.allclose(
t1.larray.type(promoted_dtype),
t2.larray.type(promoted_dtype),
rtol,
atol,
adjusted_rtol,
adjusted_atol,
equal_nan,
)
)
Expand Down