Skip to content

context : always use non-causal attention for encoder graphs #12447

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
Merged
merged 2 commits into from
Mar 18, 2025
Merged

context : always use non-causal attention for encoder graphs #12447

merged 2 commits into from
Mar 18, 2025
+9 −0

Conversation

ggerganov
Copy link
Member

@ggerganov
context : always use non-causal attention for encoder graphs

Verified

This commit was signed with the committer’s verified signature.
ggerganov Georgi Gerganov
GPG key ID: 449E073F9DC10735
Verified
Learn about vigilant mode
Loading
Loading status checks…
a0554c3 
ggml-ci
@ggerganov ggerganov requested a review from fairydreaming March 18, 2025 09:16
@CISC CISC linked an issue Mar 18, 2025 that may be closed by this pull request
Eval bug: b4882 broke t5 #12435
Closed
@ggerganov ggerganov mentioned this pull request Mar 18, 2025
Merged
@fairydreaming
Copy link
Collaborator

@ggerganov That won't work - the flag will be back to true during set_inputs() call when the mask is created. I think you have to move it to llama_context::encode().

@ggerganov
context : move the change to llama_context::encode()

Verified

This commit was signed with the committer’s verified signature.
ggerganov Georgi Gerganov
GPG key ID: 449E073F9DC10735
Verified
Learn about vigilant mode
Loading
Loading status checks…
29acf2c 
ggml-ci
fairydreaming
fairydreaming approved these changes Mar 18, 2025
View reviewed changes
Copy link
Collaborator

@fairydreaming fairydreaming left a comment

Choose a reason for hiding this comment

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

I confirm that it fixes the KQ mask problem for T5 encoder.

@fairydreaming fairydreaming mentioned this pull request Mar 18, 2025
Closed
@ggerganov ggerganov merged commit 8551c44 into master Mar 18, 2025
54 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Mar 19, 2025
@ggerganov @arthw
context : always use non-causal attention for encoder graphs (ggml-or…
4f03d40 
…g#12447)

* context : always use non-causal attention for encoder graphs

ggml-ci

* context : move the change to llama_context::encode()

ggml-ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fairydreaming fairydreaming

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

Successfully merging this pull request may close these issues.

Eval bug: b4882 broke t5
2 participants
@ggerganov @fairydreaming