Open antimora opened 2 months ago
@nathanielsimard remember when I first made the tests strict and had to convert every data type for correct comparison then we decided to make it less strict? Where do we want to go from here?
I think the first proposed solution is too restrictive, because not all backends support the same dtypes necessarily. And we might want to run tests in f16 at some point.
But adding separate methods for testing value equality and complete equality (w/ dtype) is a good idea.
Maybe something like .assert_values_eq(other)
which does not test dtype and .assert_eq(other)
which tests both values and dtype. We could also have inequality methods (so the same but _ne
instead of _eq
) for shorthand.
Description
We have identified an issue with our tensor operation tests, particularly in how they handle data types. This inconsistency can lead to false positives in our test suite, potentially masking real issues in our tensor operations.
Current Behavior
f64
data by default, while the test backend usesf32
.assert_eq()
method checks for data type equality, which is good for positive assertions but can lead to false positives in negative assertions.assert_eq(&expected, false)
, we can't be sure what specifically failed (data values, dimensions, or data type).Example
In
crates/burn-tensor/src/tests/ops/slice.rs
:This test fails with:
Expected Behavior
Proposed Solutions
f32
) across all tests, or explicitly specify the data type in each test.Additional Context
TensorData
refactor, which made the struct no longer generic over the element type.There are many tests that use
assert_eq(&expected, false)
which can give a false positive assertion, e.g. the data is equal but datatype is not but actually we want the data not to be equal.