isclose behavior on non-finite complex values seems incorrect

[mruberry]

For example:

np.isclose(complex(float('inf'), 0), complex(float('inf'), 0))
: False

np.isclose(complex(1, float('inf')), complex(1, float('nan')), equal_nan=True)
: True

In the first case it seems like the same number should be close to itself, and in the latter case I think the numbers should not be close since NaN is not close to anything.

Python's cmath.isclose behavior appears to demand that complex numbers containing -inf or inf must be identical to be close. It does not support equal_nan so complex numbers containing NaN are never close to other complex numbers. If equal_nan is True I would expect NaNs to act like infinities and require the numbers be identical to be "close."

[rgommers]

Thanks @mruberry, that's a clear bug. np.isclose doesn't have any complex tests, and the RuntimeWarning the above code snippet triggers hints that the implementation wasn't done with complex values in mind:

/home/rgommers/anaconda3/lib/python3.7/site-packages/numpy/core/numeric.py:2267: RuntimeWarning: invalid value encountered in multiply
  x = x * ones_like(cond)
/home/rgommers/anaconda3/lib/python3.7/site-packages/numpy/core/numeric.py:2268: RuntimeWarning: invalid value encountered in multiply
  y = y * ones_like(cond)

Python's cmath.isclose behavior appears to demand that complex numbers containing -inf or inf must be identical to be close. It does not support equal_nan so complex numbers containing NaN are never close to other complex numbers. If equal_nan is True I would expect NaNs to act like infinities and require the numbers be identical to be "close."

That sounds right.

[Qiyu8]

as far as I know, Complex Numbers cannot be compared. which means that it's unreasonable to judging whether 1+2i is close to 2+1i.

[mruberry]

as far as I know, Complex Numbers cannot be compared. which means that it's unreasonable to judging whether 1+2i is close to 2+1i.

While the complex numbers aren't part of any ordered field, which means there's not a great mathematical concept of saying that a < b, there is a well-defined and widely accepted notion of saying "a is close to b" even when both a and b are complex. See Python's cmath.isclose, for example: https://docs.python.org/3/library/cmath.html.

[seberg]

It might be a bit more complicated? Since np.isnan actually True or False, but our complex numbers can have only one of the components being NaN. That is a slightly weird number, but means that np.nan and 1j*np.nan can easily be considered "identical" by accident. I am curious if we can rephrase isclose (to avoid np.isnan, np.isfinite, etc.) or make isclose a ufunc itself.

[mruberry]

The implementation we're proposing for PyTorch would never consider (NaN, 0) close to (0, NaN). That said, while isfinite(Complex) seems OK to define (true if neither part is -inf, inf, or NaN), isnan(Complex) may not be definable.

[seberg]

The thing is, if we make it a ufunc, or implement with something like subtract_allow_nonfinite, or even distance and distance_nonfinite could make sense (I do not like that really, so isclose as a ufunc is probably most reasonble in the long run). The thing is that complex is just one issue. In theory types like quaternions can exist and have similar subtleties.

[rgommers]

The thing is, if we make it a ufunc,

I suggest to keep this focused on behavior. We can't make it a ufunc anyway (it has non-ufunc keywords), but either way it's not really relevant to the discussion.

It might be a bit more complicated? Since np.isnan actually True or False, but our complex numbers can have only one of the components being NaN. That is a slightly weird number, but means that np.nan and 1j*np.nan can easily be considered "identical" by accident.

This may make sense, given that a complex number really is a single number, so the whole complex number should be "not a number". (nan + 0j) and (0 + nanj) can be constructed, but I'd say isnan is correct here in just saying they're both NaN.

np.nan and 1j*np.nan can easily be considered "identical" by accident.

I'd say that's fine. We're talking about "identical" for semantic purposes here, not numerical behavior.

[eric-wieser]

Just a reminder that matching the behavior of cmath.isclose exactly isn't really an option, for the same reason that today isclose(float) does not match math.isclose(float) (gh-10161).

[seberg]

@rgommers right, you can say that 1+np.nan*1j is just one well to spell complex NaN. For most code maybe that is even a good way to define it, although things may get confusing if you then do arr.real and they are completely different.

I may have been thinking a bit more of testing, where this is not what you want probably. However, testing is not as important in the sense that I do not worry about generalizing it. The reason why I mention ufuncs is that a ufunc is the best way we have find a solution without putting in complex special paths everywhere which do not generalize.