Skip to content

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Dec 21, 2022

What does this PR do?

config.num_channels is not used in some CLIP-like modeling files. Unlike previous PRs like #20596 or #20844, we use this attribute in the modeling files in this PR.

The only breaking case is when a user previously set config.num_channels=X with X !=3, which is super unlikely IMO. (Even they did so, the actual Conv2D layer still uses 3 as it is hard-coded in the current main branch)

@ydshieh ydshieh requested a review from NielsRogge December 21, 2022 08:13
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Dec 21, 2022

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

Copy link
Contributor

@NielsRogge NielsRogge left a comment

Choose a reason for hiding this comment

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

LGTM!

@ydshieh ydshieh requested a review from LysandreJik December 21, 2022 09:39
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.

Sounds good to me, I checked on the Hub and couldn't find any model that uses config.num_channels != 3.

@ydshieh ydshieh merged commit 852e7eb into main Dec 21, 2022
@ydshieh ydshieh deleted the cleanup_config_attrs_6 branch December 21, 2022 10:51
MKhalusova pushed a commit to MKhalusova/transformers that referenced this pull request Dec 28, 2022
)

Use config.num_channels in CLIP-like modeling files

Co-authored-by: ydshieh <[email protected]>
silverriver pushed a commit to silverriver/transformers that referenced this pull request Jan 6, 2023
)

Use config.num_channels in CLIP-like modeling files

Co-authored-by: ydshieh <[email protected]>
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.

5 participants