[stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` #2375

leandrolcampos · 2024-04-22T02:04:27Z

This PR enhances the assert_almost_equal function to correctly handle cases involving infinity and NaN.

According to test_assert_almost_equal added to /test/testing/test_assertion.mojo, the current implementation of assert_almost_equal results in errors in the following cases:

    alias float_type = DType.float32
    alias _inf = inf[float_type]()
    alias _nan = nan[float_type]()
    ...
    _should_succeed(
        SIMD[float_type, 2](-_inf, _inf), SIMD[float_type, 2](-_inf, _inf)
    )
    ...
    _should_fail(
        SIMD[float_type, 2](-_inf, 0.0),
        SIMD[float_type, 2](_inf, 0.0),
        rtol=0.1,
    )
    _should_fail(
        SIMD[float_type, 2](_inf, 0.0),
        SIMD[float_type, 2](0.0, 0.0),
        rtol=0.1,
    )
    ...
    _should_fail(
        SIMD[float_type, 2](_nan, 0.0),
        SIMD[float_type, 2](0.0, 0.0),
        equal_nan=True,
    )

This PR also:

Eliminates the use of and and or in the _isclose function due to the issue outlined in [BUG] The and and or operators produce incorrect results with SIMD inputs #2374.
Explicitly reduces boolean vectors to boolean scalar values instead of counting on implicit conversions for clarity.
Avoids arithmetic operations in _isclose and assert_almost_equal when the type is boolean, as these operations are not supported in this case.
Clarifies the behavior of assert_almost_equal in the docstring, highlighting differences from similar functions such as numpy.testing.assert_allclose.
Adds the inf function to utils/_numerics along with corresponding tests in test/utils/test_numerics.mojo.

stdlib/src/testing/testing.mojo

leandrolcampos · 2024-04-29T15:39:48Z

Hello @JoeLoser,

I hope this message finds you well. I wanted to check in to see if there's anything additional you need from my side to facilitate the review process.

Thank you for your time and attention.

All the best,

abduld

Nice . Thanks for this

JoeLoser · 2024-05-01T00:18:10Z

Hello @JoeLoser,

I hope this message finds you well. I wanted to check in to see if there's anything additional you need from my side to facilitate the review process.

Thank you for your time and attention.

All the best,

Sorry for the delay in reviewing this - I was on PTO/out of office last week and swamped with meetings yesterday. I just imported this internally and it'll be merged soon! Thanks so much for the contribution (and additional review from @abduld!). 🎉

JoeLoser · 2024-05-01T02:43:28Z

✅🟣 This contribution has been merged 🟣✅

Hey @leandrolcampos,

Thanks so much for the contribution! 🎉

We're moving to a new infrastructure for merging contributions to Mojo (we're using a tool called Copybara), and your contribution has now been merged into our internal copy of the Mojo Standard Library. I've added the "merged-internally" label on this PR.

The changes in this PR will appear here in the mojo repo nightly branch when we do our next outbound synchronization at the time that the next Mojo nightly is released. That should happen within the next 24 hours.

Please let me know if you have any questions or concerns.

JoeLoser · 2024-05-01T16:40:11Z

Note to self: this just got reverted due to an internal breakage on one test; we'll look into it today, but FYI.

leandrolcampos · 2024-05-01T17:53:37Z

Please let me know if there is anything I can do to help.

…lmost_equal` (#38991) [External] [stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` This PR enhances the `assert_almost_equal` function to correctly handle cases involving infinity and NaN. According to `test_assert_almost_equal` added to `/test/testing/test_assertion.mojo`, the current implementation of `assert_almost_equal` results in errors in the following cases: ```mojo alias float_type = DType.float32 alias _inf = inf[float_type]() alias _nan = nan[float_type]() ... _should_succeed( SIMD[float_type, 2](-_inf, _inf), SIMD[float_type, 2](-_inf, _inf) ) ... _should_fail( SIMD[float_type, 2](-_inf, 0.0), SIMD[float_type, 2](_inf, 0.0), rtol=0.1, ) _should_fail( SIMD[float_type, 2](_inf, 0.0), SIMD[float_type, 2](0.0, 0.0), rtol=0.1, ) ... _should_fail( SIMD[float_type, 2](_nan, 0.0), SIMD[float_type, 2](0.0, 0.0), equal_nan=True, ) ``` This PR also: - Eliminates the use of `and` and `or` in the `_isclose` function due to the issue outlined in #2374. - Explicitly reduces boolean vectors to boolean scalar values instead of counting on implicit conversions for clarity. - Avoids arithmetic operations in `_isclose` and `assert_almost_equal` when the type is boolean, as these operations are not supported in this case. - Clarifies the behavior of `assert_almost_equal` in the docstring, highlighting differences from similar functions such as `numpy.testing.assert_allclose`. - Adds the `inf` function to `utils/_numerics` along with corresponding tests in `test/utils/test_numerics.mojo`. ORIGINAL_AUTHOR=Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com> PUBLIC_PR_LINK=#2375 Co-authored-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com> Closes #2375 MODULAR_ORIG_COMMIT_REV_ID: 2e8b24461dd3279bc841877cc0167acaa104f273

JoeLoser · 2024-05-03T04:00:12Z

Please let me know if there is anything I can do to help.

Will do! I didn't have time to look into this yet, but hoping to soon (got tied up into things for the MAX 24.3 release earlier today). I suspect it's an issue you're uncovering in some of our internal code, but I'll need to root cause it in order to land this for you internally. I'll keep you updated if there's anything, but I'm not expecting anything right now 😄

leandrolcampos · 2024-05-04T00:58:15Z

Ok. I'll resolve the conflicts in the branch 🙏

Signed-off-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>

…function Signed-off-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>

leandrolcampos · 2024-05-07T02:43:15Z

Sorry for bothering you again. You may also need to update the implementation of math.isclose.

JoeLoser · 2024-05-07T02:46:17Z

Sorry for bothering you again. You may also need to update the implementation of math.isclose.

Don't feel bad at all! I'm sorry I haven't been able to land this yet internally - it's on my list to get to though. :)

modularbot · 2024-05-08T16:48:36Z

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

JoeLoser · 2024-05-08T16:53:48Z

@leandrolcampos this just merged again internally. Thanks for pushing on this and your patience! Your PR actually exposed a real bug with handling of inf in some internal code that @dukebw just fixed this morning. Appreciate it!

leandrolcampos · 2024-05-08T17:09:29Z

@JoeLoser and @dukebw, I'm happy to be able to help you and contribute to Mojo. Thanks!

…lmost_equal` (#39452) [External] [stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` This PR enhances the `assert_almost_equal` function to correctly handle cases involving infinity and NaN. According to `test_assert_almost_equal` added to `/test/testing/test_assertion.mojo`, the current implementation of `assert_almost_equal` results in errors in the following cases: ```mojo alias float_type = DType.float32 alias _inf = inf[float_type]() alias _nan = nan[float_type]() ... _should_succeed( SIMD[float_type, 2](-_inf, _inf), SIMD[float_type, 2](-_inf, _inf) ) ... _should_fail( SIMD[float_type, 2](-_inf, 0.0), SIMD[float_type, 2](_inf, 0.0), rtol=0.1, ) _should_fail( SIMD[float_type, 2](_inf, 0.0), SIMD[float_type, 2](0.0, 0.0), rtol=0.1, ) ... _should_fail( SIMD[float_type, 2](_nan, 0.0), SIMD[float_type, 2](0.0, 0.0), equal_nan=True, ) ``` This PR also: - Eliminates the use of `and` and `or` in the `_isclose` function due to the issue outlined in #2374. - Explicitly reduces boolean vectors to boolean scalar values instead of counting on implicit conversions for clarity. - Avoids arithmetic operations in `_isclose` and `assert_almost_equal` when the type is boolean, as these operations are not supported in this case. - Clarifies the behavior of `assert_almost_equal` in the docstring, highlighting differences from similar functions such as `numpy.testing.assert_allclose`. - Adds the `inf` function to `utils/_numerics` along with corresponding tests in `test/utils/test_numerics.mojo`. ORIGINAL_AUTHOR=Leandro Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com> PUBLIC_PR_LINK=#2375 Co-authored-by: Leandro Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com> Closes #2375 MODULAR_ORIG_COMMIT_REV_ID: 8e0c2394d01d12015816f242e98d6f457af9cdfd

leandrolcampos requested a review from a team as a code owner April 22, 2024 02:04

leandrolcampos force-pushed the fix/infinity-handling-assert branch 3 times, most recently from 031ccbd to d314a48 Compare April 27, 2024 15:36

soraros reviewed Apr 28, 2024

View reviewed changes

stdlib/src/testing/testing.mojo Show resolved Hide resolved

soraros reviewed Apr 28, 2024

View reviewed changes

stdlib/src/testing/testing.mojo Outdated Show resolved Hide resolved

soraros reviewed Apr 28, 2024

View reviewed changes

stdlib/src/testing/testing.mojo Outdated Show resolved Hide resolved

leandrolcampos force-pushed the fix/infinity-handling-assert branch from 2c031c9 to 6c60c39 Compare April 30, 2024 00:27

soraros mentioned this pull request Apr 30, 2024

[stdlib] SIMD conformance to EqualityComparable #2412

Open

abduld approved these changes Apr 30, 2024

View reviewed changes

JoeLoser added imported-internally Signals that a given pull request has been imported internally. merged-internally Indicates that this pull request has been merged internally labels May 1, 2024

JoeLoser removed the merged-internally Indicates that this pull request has been merged internally label May 1, 2024

leandrolcampos added 4 commits May 3, 2024 22:07

Add the inf function to utils._numerics

d2c0145

Signed-off-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>

Correctly handle infinity (and NaN) in assert_almost_equal

d5ddd0c

Signed-off-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>

Add architecture support check for bf16 in test_inf function

5592424

Signed-off-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>

Add compile-time architecture support check for bf16 in test_inf …

14f9a1b

…function Signed-off-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>

leandrolcampos force-pushed the fix/infinity-handling-assert branch from 6c60c39 to 14f9a1b Compare May 4, 2024 01:46

JoeLoser self-assigned this May 5, 2024

ematejska added the mojo-repo Tag all issues with this label label May 6, 2024

modularbot added the merged-internally Indicates that this pull request has been merged internally label May 8, 2024

JoeLoser added the merged-externally Merged externally in public mojo repo label May 8, 2024

JoeLoser closed this May 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` #2375

[stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` #2375

leandrolcampos commented Apr 22, 2024 •

edited

leandrolcampos commented Apr 29, 2024

abduld left a comment

JoeLoser commented May 1, 2024

JoeLoser commented May 1, 2024

JoeLoser commented May 1, 2024

leandrolcampos commented May 1, 2024

JoeLoser commented May 3, 2024

leandrolcampos commented May 4, 2024

leandrolcampos commented May 7, 2024

JoeLoser commented May 7, 2024

modularbot commented May 8, 2024

JoeLoser commented May 8, 2024

leandrolcampos commented May 8, 2024

[stdlib] Enhance Handling of Infinity and NaN in assert_almost_equal #2375

[stdlib] Enhance Handling of Infinity and NaN in assert_almost_equal #2375

Conversation

leandrolcampos commented Apr 22, 2024 • edited

leandrolcampos commented Apr 29, 2024

abduld left a comment

Choose a reason for hiding this comment

JoeLoser commented May 1, 2024

JoeLoser commented May 1, 2024

JoeLoser commented May 1, 2024

leandrolcampos commented May 1, 2024

JoeLoser commented May 3, 2024

leandrolcampos commented May 4, 2024

leandrolcampos commented May 7, 2024

JoeLoser commented May 7, 2024

modularbot commented May 8, 2024

JoeLoser commented May 8, 2024

leandrolcampos commented May 8, 2024

[stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` #2375

[stdlib] Enhance Handling of Infinity and NaN in `assert_almost_equal` #2375

leandrolcampos commented Apr 22, 2024 •

edited