-
Notifications
You must be signed in to change notification settings - Fork 19.4k
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
Fixes for reference before assignment… #18640
base: master
Are you sure you want to change the base?
Fixes for reference before assignment… #18640
Conversation
Thanks for the PR. Unit tests are failing and the value-add of most of these changes is highly questionable (most of it is unreachable code which will only serve to decrease our coverage). Please double check each change and only keep the useful ones. |
keras/optimizers/base_optimizer.py
Outdated
@@ -602,6 +602,8 @@ def get_config(self): | |||
learning_rate = serialization_lib.serialize_keras_object( | |||
self._learning_rate | |||
) | |||
else: | |||
learning_rate = 0.5 |
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.
Not sure what this should be?
…Close` from `test_tf_data_compatibility` ; [keras/{ops/numpy.py,saving/saving_lib.py}] `black`en
@fchollet Hmm ok I'll see what's causing these segfaults. In the meantime a lot of the patches are to solve the traceability problem, i.e., guarantee that the - for output in ds.take(1):
- output = output.numpy()
+ output = next(iter(ds)).numpy() Should I change the PR to just that fix, and make separate PR(s) for the other cases? |
Hi @SamuelMarks Can you please resolve conflicts? Thank you! |
# Conflicts: # keras/layers/preprocessing/center_crop_test.py # keras/layers/preprocessing/random_crop_test.py # keras/layers/preprocessing/random_rotation_test.py # keras/layers/preprocessing/random_zoom_test.py # keras/layers/preprocessing/resizing_test.py
@gbaned - Sure thing. Done. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18640 +/- ##
==========================================
- Coverage 78.86% 76.07% -2.80%
==========================================
Files 511 511
Lines 48658 48667 +9
Branches 8961 8958 -3
==========================================
- Hits 38375 37021 -1354
- Misses 8431 9700 +1269
- Partials 1852 1946 +94
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…rop_test,random_crop_test} ops/operation}.py] Resolve issues found through `flake8 --config setup.cfg .`
Hi @SamuelMarks Can you please resolve conflicts? Thank you! |
# Conflicts: # keras/ops/nn.py
@gbaned Done |
@sachinprasadhs No problem. Merged and resolved conflicts. |
Hi @SamuelMarks Can you please resolve conflicts? Thank you! |
@gbaned Done. |
Hi @SamuelMarks Still conflicts are appearing, can you please resolve those? Thank you! |
# Conflicts: # keras/src/saving/saving_lib.py
@gbaned Done |
Hi @SamuelMarks Can you please resolve conflicts? Thank you! |
This PR is stale because it has been open for 14 days with no activity. It will be closed if no further activity occurs. Thank you. |
# Conflicts: # keras/src/layers/preprocessing/image_preprocessing/random_contrast_test.py # keras/src/layers/preprocessing/image_preprocessing/random_crop_test.py # keras/src/trainers/compile_utils.py
@gbaned Sure thing, done |
@SamuelMarks , can you check the failing test, looks like some discrepancies in the channel axis. |
found through static analysis tooling
(more to go; or you can merge now and await further PRs; or you can request I move this to one PR per file or per module)