Skip to content
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

Apply backend.result_type to absolute, argmax, argmin, argsort, ceil, clip and dot #18548

Merged
merged 6 commits into from
Oct 5, 2023

Conversation

james77777778
Copy link
Contributor

Followed by #18482 and #18534

The corresponding unit tests have also been added.

I choose these ops that are more commonly used in the codebase.
In ceil, int64 input will become float64 output but I have changed it to backend.floatx().

The principles in my mind, which are derived from the previous discussion:

  • match the type inference of JAX
  • if the integer input becomes a floating point output, use backend.floatx()
  • try to support all dtypes in all backends (even if it introduces some overhead for checking)

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (fd5272b) 77.67% compared to head (c96ea77) 71.97%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #18548      +/-   ##
==========================================
- Coverage   77.67%   71.97%   -5.71%     
==========================================
  Files         334      334              
  Lines       32302    32354      +52     
  Branches     6297     6308      +11     
==========================================
- Hits        25092    23287    -1805     
- Misses       5631     7578    +1947     
+ Partials     1579     1489      -90     
Flag Coverage Δ
keras 71.90% <72.36%> (-5.68%) ⬇️
keras-jax 63.17% <23.68%> (-0.06%) ⬇️
keras-numpy 57.27% <39.47%> (-0.02%) ⬇️
keras-tensorflow 63.21% <43.42%> (-0.02%) ⬇️
keras-torch ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
keras/backend/jax/numpy.py 99.17% <100.00%> (+0.28%) ⬆️
keras/backend/numpy/numpy.py 99.19% <100.00%> (+0.58%) ⬆️
keras/backend/tensorflow/numpy.py 95.74% <100.00%> (+0.35%) ⬆️
keras/ops/numpy.py 95.49% <91.66%> (+0.15%) ⬆️
keras/backend/torch/numpy.py 21.03% <0.00%> (-73.83%) ⬇️

... and 35 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@james77777778
Copy link
Contributor Author

james77777778 commented Oct 5, 2023

Some tests have failed and I believe the root cause is the new release of torch 2.1.0.
I'm working on the fix.

EDITED:
please see #18550

Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Great work!

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Oct 5, 2023
@fchollet fchollet merged commit 9284cb9 into keras-team:master Oct 5, 2023
6 checks passed
@google-ml-butler google-ml-butler bot removed ready to pull Ready to be merged into the codebase kokoro:force-run labels Oct 5, 2023
@james77777778 james77777778 deleted the apply-result-type-2 branch October 5, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants