Skip to content

Adds Snapdragon X Elite and X Plus GPU scores to hardware.ts #881

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

Merged
merged 8 commits into from
Sep 6, 2024

Conversation

mrkiura
Copy link
Contributor

@mrkiura mrkiura commented Sep 1, 2024

Added:

  • Snapdragon X Elite

    • X1E-00-1DE
    • X1E-84-100
    • X1E-80-100
    • X1E-78-100
  • Snapdragon X Plus

    • X1P-64-100

Addresses: #764
Source: https://www.qualcomm.com/products/mobile/snapdragon/laptops-and-tablets/snapdragon-x-elite

@coyotte508
Copy link
Member

Even if it has an integrated GPU, I think maybe it belongs in CPUs?

Also what to do about NPU score @pcuenca / @julien-c ? They have 45 TOps in their NPU, and arguably the NPU is more important than the GPU for ML stuff.

Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

I'd consider it a GPU still - in more long-term ideally, we'd be able to use the device info they put and customise snippets and even recommend models for their tasks down the line.

Besides, down the line when NPUs are more mainstream it might make sense to add it as a separate add-on.

Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

Hey @mrkiura - thanks a ton for your contribution, please do make sure to run the linter so that the lint test passes.

If you use VS code then you can do so with the prettier extension too.

@mrkiura
Copy link
Contributor Author

mrkiura commented Sep 3, 2024

Thanks @coyotte508 @Vaibhavs10

I have updated the branch.

Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

maybe of interest to @mfuntowicz too

@mrkiura
Copy link
Contributor Author

mrkiura commented Sep 3, 2024

It seems like the Test/node and (Test/ browser) checks are failing because this URL in packages/hub/src/utils/WebBlob.spec.ts references a file from runwayml's account, but Runway is no longer maintaining a HuggingFace organization. It probably needs to be addressed in a different PR.

Cc @julien-c @coyotte508 @Vaibhavs10

@coyotte508
Copy link
Member

It seems like the Test/node and (Test/ browser) checks are failing because this URL in packages/hub/src/utils/WebBlob.spec.ts references a file from runwayml's account, but Runway is no longer maintaining a HuggingFace organization. It probably needs to be addressed in a different PR.

Cc @julien-c @coyotte508 @Vaibhavs10

yes you can ignore those tests

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

I tend to agree with @coyotte508, it somehow feels more natural to classify them as CPUs, but no strong opinion.

Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

Good to merge then, we can revisit the classification when we have a bit more candidates? ✅

@Vaibhavs10 Vaibhavs10 merged commit e4c3eae into huggingface:main Sep 6, 2024
4 checks passed
@Vaibhavs10
Copy link
Member

Thanks a lot again for the contribution @mrkiura ❤️

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.

5 participants