Skip to content

Upgrade to Transformers 4.34#475

Merged
regisss merged 19 commits into
mainfrom
transformers_4.34
Nov 7, 2023
Merged

Upgrade to Transformers 4.34#475
regisss merged 19 commits into
mainfrom
transformers_4.34

Conversation

@regisss
Copy link
Copy Markdown
Collaborator

@regisss regisss commented Oct 19, 2023

What does this PR do?

As per title.

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
Copy link
Copy Markdown

HuggingFaceDocBuilderDev commented Oct 19, 2023

The documentation is not available anymore as the PR was closed or merged.

"""
cos, sin = self.cos_sin(seq_len, past_key_values_length, position_ids, query.device, query.dtype)

# Query and key's shapes are [bs * num_heads, seq_len, dim], might need manual expansion. Ifs and elses used to
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@regisss I think we should discard this change. It leads to a significant performance drop. Also, it can handle cases where the expansion_factor > 1 even without this change

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I just removed it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@regisss From internal tests it turned out this is needed for some configurations. Without this they got graph compilation error. Please bring this change back.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done.
What about the performance drop? Was it when doing text generation or during training?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@schoi-habana We have to remove it because the performance drop is quite important for both training and inference. I don't seen any compilation error with SynapseAI 1.12.0.
@libinta Maybe something to monitor to make sure everything works as expected for the release of 1.13?

@regisss regisss marked this pull request as ready for review October 31, 2023 21:20
@regisss regisss requested a review from a user October 31, 2023 21:20
@regisss regisss requested a review from bzhu-habana as a code owner October 31, 2023 21:20
@regisss regisss merged commit 15e17dc into main Nov 7, 2023
@regisss regisss deleted the transformers_4.34 branch November 7, 2023 09:04
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.

4 participants