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

Removing HL_DS_DISTRIBUTED_ATTENTION_SEQ_DIM as it's not needed from 1.20 #1726

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bhargaveede
Copy link
Collaborator

1.20 Release

What does this PR do?

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@yafshar
Copy link
Contributor

yafshar commented Jan 28, 2025

@bhargaveede thanks. The workaround has already been removed in the next DeepSpeed update, so it is completely safe to remove it for the 1.20 release.

Copy link
Contributor

@yafshar yafshar left a comment

Choose a reason for hiding this comment

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

LGTM!

@regisss
Copy link
Collaborator

regisss commented Jan 28, 2025

@bhargaveede This is still needed for SynapseAI 1.20 right?

@yafshar
Copy link
Contributor

yafshar commented Jan 28, 2025

@bhargaveede This is still needed for SynapseAI 1.20 right?

The workaround in DeepSpeed for SynapseAI 1.19.0 included the HL_DS_DISTRIBUTED_ATTENTION_SEQ_DIM environment variable. However, this implementation has been corrected, and the variable has been removed in the subsequent DeepSpeed release for SynapseAI 1.20. So the variable has no effect in 1.20

@regisss
Copy link
Collaborator

regisss commented Jan 28, 2025

@bhargaveede This is still needed for SynapseAI 1.20 right?

The workaround in DeepSpeed for SynapseAI 1.19.0 included the HL_DS_DISTRIBUTED_ATTENTION_SEQ_DIM environment variable. However, this implementation has been corrected, and the variable has been removed in the subsequent DeepSpeed release for SynapseAI 1.20. So the variable has no effect in 1.20

Thanks for the explanation! I meant "still needed for 1.19" actually, which is the case if I understand correctly. I'll probably merge this PR in the release branch first since I prefer keeping the main branch compatible with the latest Synapse release.

@bhargaveede
Copy link
Collaborator Author

@regisss Flag is needed for 1.19.
Only from 1.20, We don't need the flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants