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 explicit simd implementation for one pass svd in large action spaces. #4261

Merged
merged 23 commits into from
Dec 1, 2022

Conversation

zwd-ms
Copy link
Collaborator

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

Add an explicit SIMD implementation using AVX-512 for the one pass svd part in large actions. For now, it only works with quadratic interactions.

By default, VW builds with VW_BUILD_LAS_WITH_SIMD=ON so it includes the SIMD binaries. The compile flag can be turned off, for example when some CI runs on unsupported architectures (or when someone building on really old platforms), but all tests should build and pass on a recent CPU.

To enable vectorization at runtime, pass in --explicit_simd on vw command line. This flag is advisory: it only works if the program detects that the platform actually supports the needed instructions.

Next steps include adding alternative implementations using other extension sets (e.g. AVX2), and adding automatic switching between scalar and simd code paths using CPUID info at runtime, instead of using the command-line flag.

@zwd-ms zwd-ms changed the title [wip] Las svd simd [feat] Add explicit simd implementation for one pass svd in large action spaces. Nov 11, 2022
@zwd-ms zwd-ms changed the title [feat] Add explicit simd implementation for one pass svd in large action spaces. feat: Add explicit simd implementation for one pass svd in large action spaces. Nov 11, 2022
@zwd-ms zwd-ms marked this pull request as ready for review November 11, 2022 22:27
@zwd-ms zwd-ms requested a review from olgavrou November 14, 2022 14:05
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.

A separate CI job should be added that turns on the compile time flag and runs the related tests (like we had for LAS while it was behind a compile time flag). It can run only for linux architectures

@zwd-ms
Copy link
Collaborator Author

zwd-ms commented Nov 14, 2022

A separate CI job should be added that turns on the compile time flag and runs the related tests (like we had for LAS while it was behind a compile time flag). It can run only for linux architectures

Good idea. I'm adding a CI in a subsequent PR that handles the runtime detection of supported instructions. Before that's done, the tests in CI won't always pass due to "illegal instructions" when the CI runs on unsupported machines.
Update: after adding the cpu feature detection, the compile flag is set to ON by default, so we can use the existing CI for tests now.

@zwd-ms zwd-ms requested a review from lokitoth November 16, 2022 14:53
@zwd-ms zwd-ms requested a review from jackgerrits November 28, 2022 14:09
@zwd-ms zwd-ms merged commit 3ee9665 into VowpalWabbit:master Dec 1, 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.

2 participants