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

Adds tests and fixes issues with CompileLoss true/pred/loss ordering #20362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gustavoeb
Copy link

This PR addresses the issue described here: #20346

  • Firstly improved the existing test_list_loss_dict_data; the previously chosen values of a and b are such that no matter the order of the losses (mae, mse) the total loss value is always the same, hiding potential issues.
  • Added a couple more tests to mimic what happens with functional models with multiple outputs, fitted with data coming from a dictionary.
  • Keeps dicts in their existing order, or output_name order, by casting them to OrderDict to circumvent optree’s sorting behaviour

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 57.43%. Comparing base (3d32481) to head (fb04f63).

Files with missing lines Patch % Lines
keras/src/trainers/compile_utils.py 85.71% 2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (3d32481) and HEAD (fb04f63). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (3d32481) HEAD (fb04f63)
keras 4 1
keras-torch 1 0
keras-tensorflow 1 0
keras-jax 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #20362       +/-   ##
===========================================
- Coverage   78.87%   57.43%   -21.45%     
===========================================
  Files         512      512               
  Lines       49266    49280       +14     
  Branches     7953     7958        +5     
===========================================
- Hits        38861    28302    -10559     
- Misses       8546    19172    +10626     
+ Partials     1859     1806       -53     
Flag Coverage Δ
keras 57.43% <85.71%> (-21.32%) ⬇️
keras-jax ?
keras-numpy 57.43% <85.71%> (+<0.01%) ⬆️
keras-tensorflow ?
keras-torch ?

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants