-
Notifications
You must be signed in to change notification settings - Fork 45
Increase coverage and other niceties for the operator/elementwise tests #89
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.
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
Conversation
* Use `xps` dtype strategies where possible for better repr and dtype filtering * Helper for asserting broadcasted shapes * Helper `sh.iter_indices()` to wrap `ndindex` equivalent * Update `test_equal` with `sh.iter_indices()`
Also updates `test_logical_and`
Tuples give the impression of `in_dtype` being hetereogenous
Slices are not hashable!
Also moves `ah.int_to_dtype()` and renames it `mock_int_dtype()`
- Fix old usage of `mock_int_dtype` - Infer `in_stype` - Allow scalar `right` for `binary_assert_against_refimpl()` - Use util in `test_add`
Also `ignorer` -> `filter_`
@asmeurer Not quite ready yet, but you might wanna skim over to see if you can identify any issues with the "reference implementation" approach I'm using here... it's a big change 😅 Notably this does still pass-but-filter special cases (and without using boolean masks). I wrote a high-level note here: array-api-tests/array_api_tests/test_operators_and_elementwise_functions.py Lines 45 to 65 in 9d1f4da
|
Let's make sure we document that we're doing this and that any errors should usually mean the function is wrong, but it could also mean our tolerances need to be loosened. |
My biggest worry here is float32, since the reference implementation will operate on float64. And if we ever add a 16-bit float, that will be even worse. You might want to test this on something like cupy just to see if it works on an accelerator. |
# when a function should return intergrals. | ||
|
||
|
||
def isclose(a: float, b: float, rel_tol: float = 0.25, abs_tol: float = 1) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess since we are testing one float at a time, just using math.isclose here is fine. In general though, we will want an isclose, and corresponding assert_approximately_equal method that only use the array API methods so that they can be called on arrays (I will need this for the linalg tests).
I don't think it will be an issue, but if we do run into issues with float32 and the fact that Python float
is 64-bit only, we may need to make this use array operations too. That would also give us more control over the exact formula used for "closeness", though I doubt that will be an issue either given our very loose tolerances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general though, we will want an isclose, and corresponding assert_approximately_equal method that only use the array API methods so that they can be called on arrays (I will need this for the linalg tests).
The *_assert_against_refimpl
utils do this along an array/arrays, only using the Array API endpoints of indexing and scalar casting. I've kept this in just the elwise/op file for now as it's quite particular to those tests, but you might want to draw inspiration from it.
I don't think it will be an issue, but if we do run into issues with float32 and the fact that Python float is 64-bit only, we may need to make this use array operations too.
Ah I see, good to keep in mind.
Keeps all refimpl logic near eachother
Happy with this PR now. |
This PR:
ndindex.iter_indices()
to greatly improve values testing*_assert_against_refimpl()
utilsAdditionally:
sh.fmt_idx()