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

Bias tensors #1259

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Bias tensors #1259

wants to merge 3 commits into from

Conversation

gabe-l-hart
Copy link
Contributor

@gabe-l-hart gabe-l-hart commented Oct 3, 2024

Dependencies

This PR is part of a sequence in support of adding Granite Code. It depends on merging the following PRs:

Issues

Closes #1250

Description

This PR adds support for models which have bias tensors for the attention and ffn modules alongside the primary weight tensors.

Changes

  • Add the bias tensors to the weight_map in HF checkpoint conversion
  • Handle merged wqkv tensors for bias as well as weights in HF checkpoint conversion
    • This includes changes to the permutation logic to support the shapes of the bias tensors. I leveraged the corresponding logic in llama.cpp's converter.
  • Add configs to TransformerArgs to allow models to indicate the presence of attention_bias and feed_forward_bias tensors
  • Populate the Attention and FeedForward modules' tensors' bias arguments based on the config args

Testing

In conjunction with my other changes for Granite Code, I've been able to validate that the results produced with this logic do produce the expected token sequence.

NOTE: If there's any preferred way to include unit tests along with the PR, please let me know and I can get them added! I don't see a familiar unit test structure in the project at this point, so I've been relying on local ad-hoc testing.

Copy link

pytorch-bot bot commented Oct 3, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1259

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit bbea338 with merge base 766bee9 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 3, 2024
@gabe-l-hart gabe-l-hart force-pushed the BiasTensors-1250 branch 2 times, most recently from 4c08ee4 to 964ae69 Compare October 4, 2024 15:29
Branch: GraniteCodeSupport

Signed-off-by: Gabe Goodhart <[email protected]>
Branch: GraniteCodeSupport

Signed-off-by: Gabe Goodhart <[email protected]>
@gabe-l-hart gabe-l-hart marked this pull request as ready for review October 4, 2024 20:02
@gabe-l-hart
Copy link
Contributor Author

Thanks for the review/merge on #1255! This PR is now ready for review

@mikekg
Copy link

mikekg commented Oct 4, 2024

Current tests are run through .github/workflows, with some scripts in .ci (including a script that can be used to ensure that code in
markdown files works).

Or were you looking for "unit tests" of subcomponents with a Python driver? If you are looking for python-level unit tests, I don't think we have any right now, but that doesn't mean we can't have any. If you want to make a proposal, you might discuss with @byjlw and @Jack-Khuu, and @lessw2020 for distributed inference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for separate bias tensors
3 participants