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

Fix ConvNeXt classifier activation bug #17544

Merged
merged 10 commits into from
Feb 14, 2023
Merged

Fix ConvNeXt classifier activation bug #17544

merged 10 commits into from
Feb 14, 2023

Conversation

Frightera
Copy link
Contributor

Fixes the issue keras-team/tf-keras#319.

Please find the bug, fix, and test gist here.

@Frightera Frightera changed the title Pass classifier_activation arg to "Head" Fix ConvNeXt classifier activation bug Feb 9, 2023
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Feb 9, 2023
@gbaned gbaned requested a review from qlzh727 February 9, 2023 17:14
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Feb 9, 2023
Copy link
Member

@qlzh727 qlzh727 left a comment

Choose a reason for hiding this comment

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

Can you also add a unit test for this?

keras/applications/convnext.py Show resolved Hide resolved
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Feb 9, 2023
Copy link
Member

@qlzh727 qlzh727 left a comment

Choose a reason for hiding this comment

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

Please also fix the format.

keras/applications/applications_test.py Outdated Show resolved Hide resolved
@Frightera
Copy link
Contributor Author

Should be fine now.

keras/applications/applications_test.py Outdated Show resolved Hide resolved
@Frightera Frightera changed the title Fix ConvNeXt classifier activation bug Fix ConvNeXt/RegNet classifier activation bug Feb 10, 2023
@Frightera Frightera changed the title Fix ConvNeXt/RegNet classifier activation bug Fix ConvNeXt and RegNet classifier activation bug Feb 10, 2023
@grasskin grasskin removed the keras-team-review-pending Pending review by a Keras team member. label Feb 10, 2023
@Frightera Frightera changed the title Fix ConvNeXt and RegNet classifier activation bug Fix ConvNeXt classifier activation bug Feb 10, 2023
@Frightera
Copy link
Contributor Author

@qlzh727 It looks like this is ready to pull, if there is any other thing please let me know. Thanks!

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Feb 13, 2023
@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Feb 13, 2023
copybara-service bot pushed a commit that referenced this pull request Feb 14, 2023
Imported from GitHub PR #17544

Fixes the issue #17386.

Please find the bug, fix, and test [gist here](https://colab.research.google.com/gist/Frightera/93a784fc0e03a19f5804e7e14bfbecba/-17386.ipynb).
Copybara import of the project:

--
eedabb6 by Kaan <46622558+Frightera@users.noreply.github.com>:

Pass classifier_activation arg to "Head"
--
e226091 by Kaan <46622558+Frightera@users.noreply.github.com>:

Add classifier_activation to the docstring
--
3856fed by Kaan <46622558+Frightera@users.noreply.github.com>:

Add classifier_activation unit test
--
c3dfc34 by Kaan <46622558+Frightera@users.noreply.github.com>:

Move classifier_activation validation before head creation
--
71eaa69 by Kaan <46622558+Frightera@users.noreply.github.com>:

Update test_application_classifier_activation
--
ebd6940 by Kaan Bıçakcı <kaan.dvlpr@gmail.com>:

Reformatting using format.sh

--
bce4ac9 by Kaan Bıçakcı <kaan.dvlpr@gmail.com>:

Fix test_application_classifier_activation test

--
00d4889 by Kaan Bıçakcı <kaan.dvlpr@gmail.com>:

Fix Head params to accept classifier_activation

--
2c22d37 by Kaan Bıçakcı <kaan.dvlpr@gmail.com>:

Revert "Fix Head params to accept classifier_activation"

This reverts commit 00d4889.

--
3abd441 by Kaan Bıçakcı <kaan.dvlpr@gmail.com>:

Exclude RegNet in test_application_classifier_activation

Merging this change closes #17544

FUTURE_COPYBARA_INTEGRATE_REVIEW=#17544 from Frightera:frightera_fix_17386 3abd441
PiperOrigin-RevId: 509550999
@copybara-service copybara-service bot merged commit f862412 into keras-team:master Feb 14, 2023
PR Queue automation moved this from Approved by Reviewer to Merged Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to pull Ready to be merged into the codebase size:S
Projects
PR Queue
Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants