Fix NumPy normalize shape for 1D inputs - #23428
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the normalize utility to use np.asarray instead of np.atleast_1d for NumPy inputs, and adds a test to ensure shape preservation for 1D arrays. The review feedback highlights that 0D inputs still face shape-preservation issues when axis is None due to np.expand_dims being applied to the norm. It suggests a code adjustment to directly return x / norm in this scenario, along with adding a corresponding test case to prevent regressions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #23428 +/- ##
==========================================
+ Coverage 84.91% 85.02% +0.10%
==========================================
Files 468 468
Lines 71091 71091
Branches 11788 11788
==========================================
+ Hits 60368 60446 +78
+ Misses 7717 7632 -85
- Partials 3006 3013 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The TensorFlow CPU job reached the two-hour workflow limit and was cancelled; the NumPy, JAX, JAX NNX, Torch, OpenVINO, formatting, security, and CLA checks passed. I cannot rerun upstream Actions jobs from the fork, so could a maintainer please rerun the failed job when convenient? |
hertschuh
left a comment
There was a problem hiding this comment.
Thanks for addressing this!
| if isinstance(x, np.ndarray): | ||
| # NumPy input | ||
| norm = np.atleast_1d(np.linalg.norm(x, order, axis)) | ||
| norm = np.asarray(np.linalg.norm(x, order, axis)) |
There was a problem hiding this comment.
Remove np.asarray, it doesn't do anything.
norm = np.linalg.norm(x, order, axis)
There was a problem hiding this comment.
Removed np.asarray in f5348f6. norm now comes directly from np.linalg.norm, and scalar-safe zero replacement uses np.where. All 16 numerical utility tests pass with both the NumPy and JAX backends.
Signed-off-by: aswanth-07 <a.aswanth0707@gmail.com>
b5fdf68 to
f5348f6
Compare
|
@hertschuh The requested np.asarray removal is in f5348f6, and the affected numerical utility tests pass with both the NumPy and JAX backends. When convenient, could you take another look? Thanks. |
Description
keras.utils.normalize()currently changes a one-dimensional NumPy input from shape(n,)to(1, n), while the backend-tensor path correctly preserves(n,).For a 1D input,
np.linalg.normreturns a scalar. Wrapping that scalar withnp.atleast_1dgives it shape(1,); the subsequentnp.expand_dimsproduces(1, 1), and NumPy broadcasting adds an unintended leading dimension to the result.This change keeps the scalar or array returned by
np.linalg.norm, replaces zero norms withnp.where, and returnsx / normdirectly whenaxis=None. For an explicit axis, expanding the denominator along that axis preserves the one-dimensional input shape.The regression test covers
axis=-1,axis=0, andaxis=Noneand checks both the output shape and normalized values.Validation
KERAS_BACKEND=numpy python -m pytest keras/src/utils/numerical_utils_test.py -q(16 passed)KERAS_BACKEND=jax python -m pytest keras/src/utils/numerical_utils_test.py -q(16 passed)python -m ruff check keras/src/utils/numerical_utils.py keras/src/utils/numerical_utils_test.py(passed)python -m ruff format --check keras/src/utils/numerical_utils.py keras/src/utils/numerical_utils_test.py(passed)git diff --check(passed)AI assistance disclosure
OpenAI Codex assisted with repository auditing, reproduction, implementation, upstream synchronization, and test execution. The reported tests were run against the final diff.
Contributor Agreement
Please review the AI-Assisted Contribution Policy and confirm these before marking the PR ready: