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

Add name to MobileNetV3 to allow multiple instances in one model #19695

Merged

Conversation

jeffcarp
Copy link
Collaborator

@jeffcarp jeffcarp commented May 9, 2024

Fixes #19689

I opted to add a name arg to fix the issue since it's a similar pattern in other keras.applications models (updated: see conversation below). An alternative way is to use keras.src.utils.naming.uniquify internally.

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.47%. Comparing base (da83683) to head (14caa78).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19695      +/-   ##
==========================================
+ Coverage   78.52%   81.47%   +2.95%     
==========================================
  Files         498      498              
  Lines       45699    45769      +70     
  Branches     8446     8456      +10     
==========================================
+ Hits        35884    37292    +1408     
+ Misses       8087     6578    -1509     
- Partials     1728     1899     +171     
Flag Coverage Δ
keras 81.32% <100.00%> (+2.95%) ⬆️
keras-jax 64.89% <100.00%> (+2.91%) ⬆️
keras-numpy 56.45% <0.00%> (+0.13%) ⬆️
keras-tensorflow 66.37% <100.00%> (+2.95%) ⬆️
keras-torch 64.94% <100.00%> (+2.90%) ⬆️
keras.applications 80.45% <100.00%> (?)
keras.applications-jax 80.45% <100.00%> (?)
keras.applications-numpy 23.00% <0.00%> (?)
keras.applications-tensorflow 80.45% <100.00%> (?)

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.

@jeffcarp jeffcarp force-pushed the mobilenetv3-duplicate-names-19689 branch from 32e4816 to b44ade6 Compare May 9, 2024 19:01
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!

keras/src/applications/mobilenet_v3.py Outdated Show resolved Hide resolved
@fchollet
Copy link
Member

Here's my change: 2b7120b

Please apply the same for MobileNet models 👍

@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation May 10, 2024
@jeffcarp jeffcarp force-pushed the mobilenetv3-duplicate-names-19689 branch from b44ade6 to b0f4708 Compare May 11, 2024 04:50
@jeffcarp jeffcarp force-pushed the mobilenetv3-duplicate-names-19689 branch from b0f4708 to 14caa78 Compare May 11, 2024 04:53
@jeffcarp jeffcarp changed the title Add model_name to MobileNetV3 to allow multiple instances in one model Add name to MobileNetV3 to allow multiple instances in one model May 11, 2024
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.

LGTM, thank you! Can you do the same for MobileNetV2 and V1?

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer May 12, 2024
@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels May 12, 2024
@fchollet fchollet merged commit 3404ed6 into keras-team:master May 12, 2024
8 checks passed
PR Queue automation moved this from Approved by Reviewer to Merged May 12, 2024
@google-ml-butler google-ml-butler bot removed ready to pull Ready to be merged into the codebase kokoro:force-run labels May 12, 2024
jeffcarp added a commit to jeffcarp/keras that referenced this pull request May 13, 2024
@jeffcarp
Copy link
Collaborator Author

Sent #19712 for V1 and V2.

jeffcarp added a commit to jeffcarp/keras that referenced this pull request May 13, 2024
fchollet pushed a commit that referenced this pull request May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
PR Queue
Merged
Development

Successfully merging this pull request may close these issues.

Unable to make two instances of the MobileNetV3 within the same model
4 participants