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.

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

remove deprecated attn_fn #180

Closed
wants to merge 1 commit into from
Closed

remove deprecated attn_fn #180

wants to merge 1 commit into from

Conversation

samsja
Copy link
Collaborator

@samsja samsja commented Dec 20, 2024

No description provided.

@samsja samsja mentioned this pull request Dec 20, 2024
Merged
@Jackmin801
Copy link
Member

If its not too hard to recall, why did we deprecate this?
What do you think about undeprecating it? I think it can be pretty useful to swap attn impls so id prefer to keep it

Jackmin801
Jackmin801 requested changes Dec 21, 2024
View reviewed changes
Copy link
Member

@Jackmin801 Jackmin801 left a comment

Choose a reason for hiding this comment

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

Hrmm I dont think we should deprecate this

@samsja
Copy link
Collaborator Author

samsja commented Dec 23, 2024

If its not too hard to recall, why did we deprecate this? What do you think about undeprecating it? I think it can be pretty useful to swap attn impls so id prefer to keep it

as of today we only have flex_attention or sdpa. The flash attention one has been removed. To dispatch to flex or sdpa is based on the sequence_pakcing parameters

@samsja
Copy link
Collaborator Author

samsja commented Jan 5, 2025

closing as we decided to use this flag

@Jackmin801 Jackmin801 closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jackmin801 Jackmin801 Jackmin801 requested changes

@JohannesHa JohannesHa Awaiting requested review from JohannesHa

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

Successfully merging this pull request may close these issues.
2 participants
@samsja @Jackmin801