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.

Sign up for GitHub

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

Jump to bottom

[BUG] The and and or operators produce incorrect results with SIMD inputs #2374

Closed
leandrolcampos opened this issue Apr 22, 2024 · 3 comments
Closed

[BUG] The and and or operators produce incorrect results with SIMD inputs #2374

leandrolcampos opened this issue Apr 22, 2024 · 3 comments
Labels
bug Something isn't working mojo-repo Tag all issues with this label

Comments

@leandrolcampos
Copy link
Contributor

leandrolcampos commented Apr 22, 2024 •
edited

Bug description

The and and or operators produce incorrect results when they are applied to SIMD inputs.

Steps to reproduce

Run the following program:

fn main():
    var a = SIMD[DType.bool, 4](True, True, False, False)
    var b = SIMD[DType.bool, 4](True, False, False, False)

    print("and:", a and b)
    print("  &:", a & b)

    print("")
    print(" or:", a or b)
    print("  |:", a | b)

The program prints:

and: [True, True, False, False]
  &: [True, False, False, False]

 or: [True, False, False, False]
  |: [True, True, False, False]

The outputs of & and | are correct.

System information

Ubuntu 22.04.3 LTS
mojo 2024.4.1618 (37a3e965)
modular 0.7.0 (487c2f88)
@leandrolcampos leandrolcampos added bug Something isn't working mojo Issues that are related to mojo labels Apr 22, 2024
@leandrolcampos leandrolcampos changed the title [BUG] The operators and and or produces wrong results with SIMD inputs [BUG] The and and or operators produces wrong results with SIMD inputs Apr 22, 2024
@leandrolcampos leandrolcampos changed the title [BUG] The and and or operators produces wrong results with SIMD inputs [BUG] The and and or operators produce wrong results with SIMD inputs Apr 22, 2024
@leandrolcampos leandrolcampos mentioned this issue Apr 22, 2024
Closed
@leandrolcampos leandrolcampos changed the title [BUG] The and and or operators produce wrong results with SIMD inputs [BUG] The and and or operators produce incorrect results with SIMD inputs Apr 22, 2024
@VMois
Copy link

VMois commented Apr 22, 2024 •
edited

I can reproduce. Mojo 24.2.1

@soraros
Copy link
Contributor

soraros commented Apr 28, 2024

a and b is really a if bool(a) else b, which can be confusing. We are moving away from Boolable SIMD when size > 1.

@linear linear bot removed the mojo Issues that are related to mojo label Apr 29, 2024
@linear linear bot changed the title [BUG] The and and or operators produce incorrect results with SIMD inputs [BUG] The and and or operators produce incorrect results with SIMD inputs Apr 29, 2024
@ematejska ematejska added the mojo-repo Tag all issues with this label label Apr 29, 2024
@leandrolcampos
Copy link
Contributor Author

I'm closing this issue because the same subject was discussed in #252 and #275.

JoeLoser pushed a commit to JoeLoser/mojo that referenced this issue May 3, 2024
@modularbot @leandrolcampos
[External] [stdlib] Enhance Handling of Infinity and NaN in `assert_a…
0d601ee 
…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 modularml#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=modularml#2375

Co-authored-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>
Closes modularml#2375
MODULAR_ORIG_COMMIT_REV_ID: 2e8b24461dd3279bc841877cc0167acaa104f273
JoeLoser pushed a commit that referenced this issue May 3, 2024
@modularbot @leandrolcampos @JoeLoser
[External] [stdlib] Enhance Handling of Infinity and NaN in `assert_a…
d602ead 
…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 pushed a commit to JoeLoser/mojo that referenced this issue May 8, 2024
@modularbot @leandrolcampos
[External] [stdlib] Enhance Handling of Infinity and NaN in `assert_a…
8af5898 
…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 modularml#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=modularml#2375

Co-authored-by: Leandro Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>
Closes modularml#2375
MODULAR_ORIG_COMMIT_REV_ID: 8e0c2394d01d12015816f242e98d6f457af9cdfd
JoeLoser pushed a commit that referenced this issue May 8, 2024
@modularbot @leandrolcampos @JoeLoser
[External] [stdlib] Enhance Handling of Infinity and NaN in `assert_a…
b2b8032 
…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
lsh pushed a commit to lsh/mojo that referenced this issue May 17, 2024
@modularbot @lsh
[External] [stdlib] Enhance Handling of Infinity and NaN in `assert_a…
1051379 
…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 modularml#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=modularml#2375

Co-authored-by: Leandro Augusto Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>
Closes modularml#2375
MODULAR_ORIG_COMMIT_REV_ID: 2e8b24461dd3279bc841877cc0167acaa104f273

Signed-off-by: Lukas Hermann <lukashermann28@gmail.com>
lsh pushed a commit to lsh/mojo that referenced this issue May 17, 2024
@modularbot @lsh
[External] [stdlib] Enhance Handling of Infinity and NaN in `assert_a…
438c9f0 
…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 modularml#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=modularml#2375

Co-authored-by: Leandro Lacerda Campos <15185896+leandrolcampos@users.noreply.github.com>
Closes modularml#2375
MODULAR_ORIG_COMMIT_REV_ID: 8e0c2394d01d12015816f242e98d6f457af9cdfd

Signed-off-by: Lukas Hermann <lukashermann28@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working mojo-repo Tag all issues with this label
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@soraros @leandrolcampos @ematejska @VMois