array-api-tests icon indicating copy to clipboard operation
array-api-tests copied to clipboard

test_clip: make sure `min` and `max` have the same dtype as `x`

Open ev-br opened this issue 8 months ago • 4 comments

closes gh-359, closes gh-339

Per the spec: 'should have the same dtype as x': https://data-apis.org/array-api/latest/API_specification/generated/array_api.clip.html

The spec is not very clear on whether min and max can be int scalars for a real-valued x array, should probably clarify in the spec:

  • On one hand, Hence, if x and either min or max have different data type kinds (e.g., integer versus floating-point), behavior is unspecified and thus implementation-dependent.
  • On the other hand, https://data-apis.org/array-api/latest/API_specification/type_promotion.html#type-promotion, allows int->float->float32

ev-br avatar Apr 08 '25 13:04 ev-br

Run tests with array_api_compat.{numpy,torch} and jax.numpy with 50_000 examples locally.

ev-br avatar Apr 08 '25 13:04 ev-br

Tested the branch (running with 2_000 examples) locally and can verify that it resolves gh-359 for dpctl.tensor

ndgrigorian avatar Apr 10 '25 01:04 ndgrigorian

The spec is not very clear on whether min and max can be int scalars for a real-valued x array, should probably clarify in the spec:

  • On one hand, Hence, if x and either min or max have different data type kinds (e.g., integer versus floating-point), behavior is unspecified and thus implementation-dependent.
  • On the other hand, https://data-apis.org/array-api/latest/API_specification/type_promotion.html#type-promotion, allows int->float->float32

Definitely the spec should clarify that int scalars should be cast to float.

crusaderky avatar Apr 10 '25 17:04 crusaderky

https://github.com/data-apis/array-api/issues/925

crusaderky avatar Apr 11 '25 10:04 crusaderky

Okay, let's merge this as approved for this seems to fix rather annoying failures. I've added a bullet point in https://github.com/data-apis/array-api-tests/issues/301 to track the "int min, float array" case. Thanks @ndgrigorian @crusaderky for the reviews.

ev-br avatar Apr 15 '25 09:04 ev-br