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

feat: Add avx2 implementation for one pass svd in large action spaces. #4281

Merged
merged 31 commits into from
Dec 3, 2022

Conversation

zwd-ms
Copy link
Collaborator

@zwd-ms zwd-ms commented Nov 14, 2022

No description provided.

@zwd-ms zwd-ms changed the title [wip] Las svd avx2 feat: Add avx2 implementation for one pass svd in large action spaces. Nov 30, 2022
@zwd-ms zwd-ms marked this pull request as ready for review December 1, 2022 22:03
red_features.generated_interactions ? *red_features.generated_interactions : *ex->interactions;
for (const auto& ns : interactions)
{
// TODO: other interactions, ignores & extent_interactions. The following only handles quadratics.
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we emit some sort of warning if users select LAS + simd + ignore/extend/cubics/other instead of silently ignoring?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, let me handle these and add warnings and some tests in a separate pr.

Copy link
Collaborator

@olgavrou olgavrou left a comment

Choose a reason for hiding this comment

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

approve with suggestion

@rami-lv
Copy link
Contributor

rami-lv commented Dec 2, 2022

@zwd-ms Did you benchmark the performance improvement of using avx512 I would be so glad if you can share the numbers.

@zwd-ms zwd-ms enabled auto-merge (squash) December 3, 2022 03:40
@zwd-ms
Copy link
Collaborator Author

zwd-ms commented Dec 3, 2022

Did you benchmark the performance improvement of using avx512

@rami-lv yes we see good perf speed up for an expensive part in cb_las, but for lda the computation is different and will require further investigation.

@zwd-ms zwd-ms merged commit 36f46f6 into VowpalWabbit:master Dec 3, 2022
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.

3 participants