8000 Added False case implementation for config.do_stable_layer_norm in FlaxWav2vec2Models by ctr-pmuruganTT · Pull Request #37822 · huggingface/transformers · GitHub
[go: up one dir, main page]
Skip to content

Added False case implementation for config.do_stable_layer_norm in FlaxWav2vec2Models #37822

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Added False case implementation for config.do_stable_layer_norm in FlaxWav2vec2Models #37822

wants to merge 1 commit into from

Conversation

ctr-pmuruganTT
Copy link
@ctr-pmuruganTT ctr-pmuruganTT commented Apr 28, 2025 •
edited
Loading

What does this PR do?

In FlaxWa2Vec2 there is no implementation when config.do_stable_layer_norm is false.

There are more chances that we will get this Not Implemented error frequently, since this value is false by default.

For this scenario, there is an implementation in modeling_tf_wav2vec2.py. I have recreated the same in flax version for modeling_flax_wav2vec2.py

I have added 4 functions which are the No Stable Layer version code for existing functions

  • FlaxWav2Vec2LayerNormPostAttention -> FlaxWav2Vec2StableLayerNormEncoder
  • FlaxWav2Vec2LayerNormPostAttentionCollection -> FlaxWav2Vec2EncoderLayerStableLayerNormCollection
  • FlaxWav2Vec2EncoderStableLayerNormPostAttention -> FlaxWav2Vec2EncoderLayerStableLayerNorm
  • FlaxWav2Vec2NoLayerNormConvLayer -> FlaxWav2Vec2LayerNormConvLayer

Fixes #37731

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Attaching some Screenshots for reference :

Before making this change, I am getting the error as stated in #37731

Screenshot 2025-04-28 at 10 11 04 AM

After making this change, Model is loaded and I am able to print it

Screenshot 2025-04-28 at 10 28 47 AM

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@gante and @Rocketknight1

@github-actions GitHub Actions
Copy link
Contributor

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the Ready for review button (at the bottom of the PR page). This will assign reviewers and trigger CI.

@github-actions github-actions bot marked this pull request as draft April 28, 2025 07:33
@ctr-pmuruganTT ctr-pmuruganTT marked this pull request as ready for review April 28, 2025 07:40
@github-actions github-actions bot requested a review from eustlb April 28, 2025 07:40
@gante
Copy link
Member
gante commented Apr 29, 2025

Hi @ctr-pmuruganTT 👋 Thank you for opening the PR

FYI flax+transformers is in support mode, and we're usually not adding PRs that go beyond a few LoC. My recommendation would be for you to keep a fork of transformers, with your changes on top :)

@mrakitaTT
Copy link

FYI flax+transformers is in support mode, and we're usually not adding PRs that go beyond a few LoC

Hi @gante, does this mean that for any future model no JAX implementations will be added in transformers library nor accepted as external contribution? I'm curious if you could please share any more details regarding this.

@gante
Copy link
Member
gante commented May 13, 2025

Yeah :) That has been the state for FLAX in transformers for like 2 years

(TL;DR maintaining different ML frameworks is very costly, and FLAX/TF has very low usage compared to torch)

@mrakitaTT
Copy link

I understand, thank you for the info @gante! I've noticed there haven't been many updates to the Flax models, and I guessed it has something to do with your prioritization, but I wasn't aware you also wouldn't be open to external contributions in that space :) I can understand though, it takes time from someone on your side to review/maintain the changes.

@ctr-pmuruganTT let's keep these changes on a branch and we will see how to port them to our repo, this PR can be closed

@eustlb
Copy link
Contributor
eustlb commented Jun 27, 2025

For the aforementioned reasons, closing this PR 🤗

@eustlb eustlb closed this Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eustlb eustlb Awaiting requested review from eustlb

Assignees
No one assigned
Labels
Flax
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Facing "NotImplementedError: config.do_stable_layer_norm is False is currently not supported" in FlaxWav2vec2Models
5 participants
@ctr-pmuruganTT @gante @mrakitaTT @eustlb @Rocketknight1
0