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

adding whisper large peft+int8 training example #95

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

pacman100
Copy link
Contributor

What does this PR do?

  1. Fixes [Whisper] fine-tune Whisper with int-8 (scripts + example notebook) #87 - adding whisper large peft+int8 training example notebook.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

This looks amazing! 🔥 Looks excellent to me!

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 16, 2023

View / edit / reply to this conversation on ReviewNB

sayakpaul commented on 2023-02-16T10:25:22Z
----------------------------------------------------------------

Add a small introduction section and club the code cells before datasets import under a section called "Initial setup".


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 16, 2023

View / edit / reply to this conversation on ReviewNB

sayakpaul commented on 2023-02-16T10:25:23Z
----------------------------------------------------------------

Better to define openai/whisper-large-v2 in a variable like model_ckpt and then reuse it throughout. Reduces the cognitive load.

Also, for models that include multiple modalities like this one, we usually maintain a standalone processor instead of separate feature extractors and tokenizers. I think we should use WhisperProcessor (https://huggingface.co/docs/transformers/model_doc/whisper#transformers.WhisperProcessor) here, no? An example of using the processor of a multimodal model is available here: https://huggingface.co/docs/transformers/main/tasks/image_captioning


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 16, 2023

View / edit / reply to this conversation on ReviewNB

sayakpaul commented on 2023-02-16T10:25:25Z
----------------------------------------------------------------

Line #7.    class DataCollatorSpeechSeq2SeqWithPadding:

Maybe this could later go into transformers like the other data collators we have.


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 16, 2023

View / edit / reply to this conversation on ReviewNB

sayakpaul commented on 2023-02-16T10:25:27Z
----------------------------------------------------------------

🔥


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 16, 2023

View / edit / reply to this conversation on ReviewNB

sayakpaul commented on 2023-02-16T10:25:28Z
----------------------------------------------------------------

Maybe add a sentence drawing the reader's attention to the fact that we're ONLY training 1% of the total model params.


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 16, 2023

View / edit / reply to this conversation on ReviewNB

sayakpaul commented on 2023-02-16T10:25:29Z
----------------------------------------------------------------

Put Seq2SeqTrainingArguments in Seq2SeqTrainingArguments? 


Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

तुम्ही खूप चांगले काम केले! 🔥

Maybe just format the code with something like jupyter-black so that the code reads more beautiful?

@pacman100 pacman100 merged commit 8ace553 into main Feb 16, 2023
@pacman100 pacman100 deleted the smangrul/add-whisper-example branch February 16, 2023 12:14
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.

[Whisper] fine-tune Whisper with int-8 (scripts + example notebook)
3 participants