keras
keras copied to clipboard
Updated Complex dtype input support for layers
Updated complex dtype for ALLOWED_DTYPES=complex64,complex128 in the documentation. Please have a look at the changes and do the needful. Thank you. Fixes #19860
Thanks for the PR! We'd need to add some test with a custom layer with complex inputs to verify that it works (and make sure we won't break it in the future). Also, current tests seem to be failing.
Hi, Is there any progress on this feature? Having layers with complex inputs is a crucial requirement for many areas of application. Any information on this issue would be appreciated. Thanks!
Hi @jhoydis -
I am working on adding some test with a custom layer with complex inputs. Will update with successful test once its done. Thanks..!!
Hi, thanks for looking into this - just some comment: I tried applying the proposed fix (add complex64/128 in ALLOWED_DTYPES) to activate complex type support for layers, but noticed this doesn't fix if fully: using slicing operation (I used layers.__getitem__ through [..] syntax, but believe this maps to any other tf.slice or tf.strided_slice) with complex64 type input results in float32 type output instead of expected complex64. Kind regards!
So what shall we do with this PR?
So what shall we do with this PR?
Hi @fchollet - Sorry for the delay. I am working on adding test for complex input. I'll update soon(maybe today or tomorrow), once newly added test case will get passed. Thanks..!!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 73.39%. Comparing base (
befa049) to head (c7f98d9). Report is 131 commits behind head on master.
:exclamation: There is a different number of reports uploaded between BASE (befa049) and HEAD (c7f98d9). Click for more details.
HEAD has 2 uploads less than BASE
Flag BASE (befa049) HEAD (c7f98d9) keras 4 3 keras-torch 1 0
Additional details and impacted files
@@ Coverage Diff @@
## master #19872 +/- ##
==========================================
- Coverage 79.35% 73.39% -5.96%
==========================================
Files 501 509 +8
Lines 47311 48277 +966
Branches 8689 8883 +194
==========================================
- Hits 37542 35432 -2110
- Misses 8014 11063 +3049
- Partials 1755 1782 +27
| Flag | Coverage Δ | |
|---|---|---|
| keras | 73.32% <100.00%> (-5.89%) |
:arrow_down: |
| keras-jax | 62.28% <100.00%> (-0.19%) |
:arrow_down: |
| keras-numpy | 57.43% <100.00%> (-0.15%) |
:arrow_down: |
| keras-tensorflow | 63.63% <100.00%> (-0.23%) |
:arrow_down: |
| keras-torch | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Are you doing ok with the test fixes across the different backends? I see that TF is passing at least, that's good!
Are you doing ok with the test fixes across the different backends? I see that TF is passing at least, that's good!
Hi @fchollet - For Torch backend I am not doing test fixes for complex dtype as torch doesn't support it. And for JAX backend running the failed test in my local same test went passed. But when commit it, that is failing. But as TF passing is good and custom layer with complex input case is working fine then can I leave it different backend(JAX and torch) as it is ?
For Torch backend I am not doing test fixes for complex dtype as torch doesn't support it.
Then you should explicitly exclude torch from these unit tests via a pytest skip decorator.
And for JAX backend running the failed test in my local same test went passed. But when commit it, that is failing.
What is the issue here?
Thanks for the update.
This branch has conflicts that must be resolved
Can you please solve the conflicts?
Thanks for the update.
This branch has conflicts that must be resolved
Can you please solve the conflicts?
Sure @fchollet - I'll resolve it and update the commit by tomorrow. Every test case got passes except torch(even after adding pytest skip decorator in keras/src/backend/common/dtypes_test.py file).
Thanks, I'll fix the remainder forward.
Thanks @fchollet..!!
Please see #20278 for a related issue that seems to be introduced with this merge.