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 a new param to set the self attn text context factor #2433

Conversation

WilliamTambellini
Copy link
Contributor

Add an env var to set the text context factor
WHISPER_SELFATTN_CACHE_TEXT_CTX_FACTOR
same default to 3.
Resolve:
#2334

@amoskahiga
Copy link

Thanks for the fix! Looks good to me.

@WilliamTambellini
Copy link
Contributor Author

@aderbedr FYI

Copy link

@aderbedr aderbedr left a comment

Choose a reason for hiding this comment

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

lgtm

@matiaslin
Copy link

Great! This looks good to me.

@WilliamTambellini
Copy link
Contributor Author

@ggerganov ?

@ggerganov
Copy link
Owner

It's better to avoid the environment variable. Should implement this by adding a int max_decoders parameter to whisper_context_params and using it for the factor.

…rs')

Add a param to set the text context factor.
No change of behavior: same default (3).
Resolve:
ggerganov#2334
@WilliamTambellini
Copy link
Contributor Author

ok @ggerganov
I ve reimplemented and repushed.
Please re-review.
best

@WilliamTambellini WilliamTambellini changed the title Add an env var to set the self attn text context factor Add a new param to set the self attn text context factor Oct 1, 2024
@ggerganov
Copy link
Owner

@WilliamTambellini I've pushed an alternative fix. Can you give it a try: #2443

@ggerganov
Copy link
Owner

Superseded by #2443

@ggerganov ggerganov closed this Oct 5, 2024
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