-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add an arithmetic_compat option to xr.set_options #10943
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient. |
a1af177 to
4c5a3a4
Compare
…ow non-index coordinates of the same name are compared for potential conflicts when performing binary operations. The default of compat='minimal' matches the previous behaviour.
a6a29f0 to
6f90b7c
Compare
Co-authored-by: Maximilian Roos <[email protected]>
| Parameters | ||
| ---------- | ||
| arithmetic_broadcast: bool, default: True |
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.
👀 didn't know this was an option!
| actual = alt + orig | ||
| assert_identical(expected, actual) | ||
|
|
||
| def test_math_with_arithmetic_compat_options(self) -> None: |
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.
can we add tests for datatree also please
This determines how non-index coordinates of the same name are compared for potential conflicts when performing binary operations.
The default is
arithmetic_compat='minimal', which matches the previous behaviour.I had originally planned to set this up to migrate the default to
arithmetic_compat='override'alongside the wider migration of default to `compat='override' elsewhere. However I've held off on this for now pending a decision on what to do anything about the original motivation of compat='minimal' here which was to drop clashing scalar coordinates that arise from things like arr[0] + arr[1].I think this is nice to have in general, complimenting the
arithmetic_joinsetting and making sure we have the same degree of control over this for arithmetic operations as we do for things likexr.merge. But the immediate motivation was#10924 where we need to skip equality checks on coordinates in situations where we want to JIT a function that uses jax arrays for coordinates.
whats-new.rst