macos failure fix

[Bchass]

Description

Resolves: #399

Same issue: scikit-learn/scikit-learn#10562 tests were failing only for macos with np.testing.assert_allclose.

numpy/numpy#7726 discussion on why numpy informs users that allclose is the equivalent to np.testing.assert_allclose when they provide different default values.

Another discussion: numpy/numpy#3183 (comment)

Based on both of these discussions, not much came out of them. It really comes down to maintainers to set the path forward for issues like this. It's hard to say what the core issue is exactly. One can say that it's on numpy/scikit but on the other hand one can say that it's an issue with macos.

From what I've read so far surrounding this issue, it's an issue with numpy. Not so much a bug though, more of a decision the maintainers of numpy made a long time ago.

Questions

• For future tests going forward, what should be recommended ? assert np.allclose or np.testing.assert_allclose? It's a bit unknown when it comes to macos.

  • assert np.allclose unfortunately does not give the same amount of detail as np.testing.assert_allclose.

Status

• Ready to go

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c68f8b8) 98.0% compared to head (afb861e) 98.0%.
Report is 14 commits behind head on master.

Additional details and impacted files

@@          Coverage Diff           @@
##           master    #423   +/-   ##
======================================
  Coverage    98.0%   98.0%           
======================================
  Files         156     158    +2     
  Lines        3065    3074    +9     
  Branches      742     739    -3     
======================================
+ Hits         3006    3015    +9     
  Misses         37      37           
  Partials       22      22           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[purva-thakre]

Great find @Bchass ! I did not think it would be this quick.

To answer your question, another option is to use assert abs(test_result - expected_result).all() <= 1E-3 or something like assert (test_result = expected_result).all(). Could you please check if either of the two cause any failures for macOS?

I usually rely on using .all().

I haven't had a chance to read the linked discussions in numpy and scikit. I'll add more related to this later.

Note: The description of this PR is probably something we want to keep in mind for #63

[Bchass]

Will do! I'll do some more testing with what you recommended later tonight or sometime during this long weekend.

[Bchass]

Final answer that was given from numpy: numpy/numpy#7726 (comment)

More info to read upon later: https://peps.python.org/pep-0485/#absolute-tolerance-default

Floating point numbers for macOS seem to be handled differently and this is my theory on why this one test was failing with the combo of np.testing.assert_allclose and the values being used. Research this later. Could be totally wrong on all of this.

[vprusso]

Excellent sleuthing, @Bchass! Great find! I'm all good for this one to be merged. All good on your end too, @purva-thakre ?

[purva-thakre]

I'm all good for this one to be merged

@vprusso How about we wait a little bit to check failures on the different ways to check for numpy array equality? We need to standardize this across all tests. Right now, tests use different options depending on who added the test. It's possible there are other options for this that have not been used by us.

I want to search those options and have @Bchass test these out since they have access to a macOS device.

[Bchass]

I agree with @purva-thakre to wait a bit. Want to break into some other tests. The interesting part is the test never failed locally on a mac. Only with GitHub Actions.

[Bchass]

To answer your question, another option is to use assert abs(test_result - expected_result).all() <= 1E-3 or something like assert (test_result = expected_result).all(). Could you please check if either of the two cause any failures for macOS?

assert abs(test_result - expected_result).all() <= 1E-3 passes locally and remotely. See: Bchass#3

assert (test_result = expected_result).all() fails locally.

This is only for this one test case. I'll try some more test cases after.

[purva-thakre]

The interesting part is the test never failed locally on a mac. Only with GitHub Actions.

@Bchass There was a similar issue in MItiq due to some changes I introduced to the code to satisfy mypy. The test would fail on macos. unitaryfund/mitiq#2127

@natestemen did a great job of looking for the source of the issue. His view as to why the tests passed locally on macos but not in Github actions was due to the differences in the resources available in both scenarios.

assert abs(test_result - expected_result).all() <= 1E-3 passes locally and remotely. See: Bchass#3

assert (test_result = expected_result).all() fails locally.

This is really weird though. I thought .all() would fail evenly for both scenarios. Maybe I should look more into the linked numpy discussions. Thanks again!

[Bchass]

Thanks for the link to that issue. Maybe this is following in the same realm? I'm going to explore more and see if I can dig further into why np.testing.assert_allclose only fails remotely and maybe get it to pass finally.

[purva-thakre]

@Bchass Could you please check if the following also pass or fail randomly?

• https://numpy.org/doc/stable/reference/generated/numpy.array_equal.html
• https://numpy.org/doc/stable/reference/generated/numpy.array_equiv.html
• https://numpy.org/doc/stable/reference/generated/numpy.allclose.html
• https://numpy.org/doc/stable/reference/generated/numpy.equal.html
• https://docs.pytest.org/en/latest/reference/reference.html#pytest.approx

So far, I am leaning towards using assert abs(test_result - expected_result).all() <= 1E-3 across all tests. But we'll wait for @vprusso's comment on this. Seems like comparing the difference of the elements of two arrays requires fewer resources than comparing the equality of the two.

If we use allclose, then we will have to make sure there are no weird approximation issues.

[Bchass]

@Bchass Could you please check if the following also pass or fail randomly?

• https://numpy.org/doc/stable/reference/generated/numpy.array_equal.html
• https://numpy.org/doc/stable/reference/generated/numpy.array_equiv.html
• https://numpy.org/doc/stable/reference/generated/numpy.allclose.html
• https://numpy.org/doc/stable/reference/generated/numpy.equal.html
• https://docs.pytest.org/en/latest/reference/reference.html#pytest.approx

So far, I am leaning towards using assert abs(test_result - expected_result).all() <= 1E-3 across all tests. But we'll wait for @vprusso's comment on this. Seems like comparing the difference of the elements of two arrays requires fewer resources than comparing the equality of the two.

If we use allclose, then we will have to make sure there are no weird approximation issues.

All pass locally/remotely. assert_allclose fails due to specific indices given in the test case. A few days ago assert_allclose passed in my remote branch when supplying whole numbers and not using the specific indices.

[purva-thakre]

Then, let's stick to using assert abs(test_result - expected_result).all() <= 1E-3.

@vprusso Thought on this?

[purva-thakre]

Going to go ahead and merge this PR. Thanks @Bchass !