Return nan from jnp.median on an empty reduction axis - #40320
Open
teddytennant wants to merge 1 commit into
Open
Return nan from jnp.median on an empty reduction axis#40320teddytennant wants to merge 1 commit into
teddytennant wants to merge 1 commit into
Conversation
Collaborator
|
I wonder if we really want to mimic NumPy's behavior here? When would it be useful to silently get an array of NaNs instead of getting a hard error during tracing? The current behavior seems much safer to me. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jnp.medianraises an internal gather error when the reduction axis is empty, where NumPy returns nan:Same for
axis=None, tuple axes, negative axes andkeepdims=True.jnp.nanmedianalready returns nan, so the two disagree today.#39475 added an empty-reduction-axis early return to
_quantile, but gated it onsquash_nans, which only thenan*entry points set.mediangoes throughquantile(a, 0.5, method='midpoint')withsquash_nans=Falseand falls through to the sort and gather, where the gather shape rule rejects a slice size of 1 against a dimension of size 0.This adds a
nan_if_emptyflag to_quantileand hasmedianpass it.mediannow calls_quantiledirectly, because the flag is not somethingjnp.quantileshould expose.nanmedianis unchanged: it routes throughnanquantile, which already has the right empty behavior. One side effect worth flagging: the error formedian(x, overwrite_input=True)andmedian(x, out=...)now saysjax.numpy.median does not support ...instead ofjax.numpy.quantile does not support ....quantileandpercentileare deliberately left alone. You asked on #39475 for them to be covered too, but NumPy only returns nan formedian; forquantileandpercentileNumPy raisesIndexError, and JAX raises as well, so the only thing left to change there is which error, which is a design call rather than a bug fix. Happy to fold that into this PR if you want it,IndexErroror otherwise.Tests:
testMedianEmptyReductionintests/lax_numpy_reducers_test.py, checked against NumPy over 1-D, 2-D and 3-D empty inputs,axis=None, int, negative and tuple axes,keepdimsboth ways, eager and jit, plus two cases where the array is empty but the reduction axis is not.With the source change reverted and the test kept, 20 of the 22 cases fail; the 2 that pass are the non-empty-reduction-axis controls. CPU only, I have no GPU or TPU to test on.