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 ConvNeXT #15277

Merged
merged 27 commits into from Feb 7, 2022
Merged

Add ConvNeXT #15277

merged 27 commits into from Feb 7, 2022

Conversation

NielsRogge
Copy link
Contributor

@NielsRogge NielsRogge commented Jan 21, 2022

What does this PR do?

This PR adds ConvNeXT to the library, a convnet inspired by Transformers.

To do:

  • rename nielsr to facebook to fix tests
  • remove inference.py script
  • remove PushToHubMixin hack from feature extractor
  • fix "gamma" in from_pretrained method. Right now, if a model includes a parameter named "gamma", they are not initialized from the hub, due to this line.
  • discuss model output classes. It feels a bit weird to talk about "hidden states" for convolutional models, most people use the term "features" or "feature maps". These are of shape (batch_size, num_channels, a certain height, a certain width) at each stage, rather than (batch_size, seq_len, hidden_size) at each layer of a Transformer. For now, they are still called hidden_states in this PR.

@HuggingFaceDocBuilder
Copy link

HuggingFaceDocBuilder commented Jan 21, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Great new addition!

docs/source/model_doc/convnext.mdx Outdated Show resolved Hide resolved
docs/source/model_doc/convnext.mdx Outdated Show resolved Hide resolved
src/transformers/__init__.py Outdated Show resolved Hide resolved
src/transformers/__init__.py Outdated Show resolved Hide resolved
src/transformers/__init__.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
tests/test_modeling_convnext.py Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the addition, looks great! I've added a couple of comments

src/transformers/models/convnext/configuration_convnext.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

On top of the other comments, just two nits from my part

docs/source/model_doc/convnext.mdx Outdated Show resolved Hide resolved
src/transformers/models/convnext/modeling_convnext.py Outdated Show resolved Hide resolved
@NielsRogge NielsRogge merged commit 84eec9e into huggingface:master Feb 7, 2022
stevhliu pushed a commit to stevhliu/transformers that referenced this pull request Feb 18, 2022
* First draft

* Add conversion script

* Improve conversion script

* Improve docs and implement tests

* Define model output class

* Fix tests

* Fix more tests

* Add model to README

* Apply suggestions from code review

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

* Apply more suggestions from code review

* Apply suggestions from code review

* Rename dims to hidden_sizes

* Fix equivalence test

* Rename gamma to gamma_parameter

* Clean up conversion script

* Add ConvNextFeatureExtractor

* Add corresponding tests

* Implement feature extractor correctly

* Make implementation cleaner

* Add ConvNextStem class

* Improve design

* Update design to also include encoder

* Fix gamma parameter

* Use sample docstrings

* Finish conversion, add center cropping

* Replace nielsr by facebook, make feature extractor tests smaller

* Fix integration test

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
ManuelFay pushed a commit to ManuelFay/transformers that referenced this pull request Mar 31, 2022
* First draft

* Add conversion script

* Improve conversion script

* Improve docs and implement tests

* Define model output class

* Fix tests

* Fix more tests

* Add model to README

* Apply suggestions from code review

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

* Apply more suggestions from code review

* Apply suggestions from code review

* Rename dims to hidden_sizes

* Fix equivalence test

* Rename gamma to gamma_parameter

* Clean up conversion script

* Add ConvNextFeatureExtractor

* Add corresponding tests

* Implement feature extractor correctly

* Make implementation cleaner

* Add ConvNextStem class

* Improve design

* Update design to also include encoder

* Fix gamma parameter

* Use sample docstrings

* Finish conversion, add center cropping

* Replace nielsr by facebook, make feature extractor tests smaller

* Fix integration test

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants