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

Add arm64 check when SIMD is disabled #1618

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

ryanbogan
Copy link
Member

@ryanbogan ryanbogan commented Apr 17, 2024

Description

There is currently a bug when attempting to run k-NN plugin with arm64 and simd disabled, where the cmake command fails and integration tests cannot be run. This PR adds a check similar to the existing aarch64 check that allows the command to complete successfully.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Ryan Bogan <[email protected]>
CHANGELOG.md Outdated
@@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
* Implemented the Streaming Feature to stream vectors from Java to JNI layer to enable creation of larger segments for vector indices [#1604](https://github.com/opensearch-project/k-NN/pull/1604)
* Remove unnecessary toString conversion of vector field and added some minor optimization in KNNCodec [1613](https://github.com/opensearch-project/k-NN/pull/1613)
### Bug Fixes
* Add arm64 check when SIMD is disabled [#1618](https://github.com/opensearch-project/k-NN/pull/1618)
Copy link
Member

Choose a reason for hiding this comment

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

I would say this is infrastructure as it doesnt impact prod code

jmazanec15
jmazanec15 previously approved these changes Apr 18, 2024
@ryanbogan
Copy link
Member Author

Windows failure not related to this change.
Neither is BWC failure: #1622

@ryanbogan ryanbogan merged commit a2bb6cc into opensearch-project:main Apr 18, 2024
46 of 51 checks passed
@ryanbogan ryanbogan deleted the fix_cmake_bug branch April 18, 2024 18:52
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 18, 2024
* Add arm64 check when SIMD is disabled

Signed-off-by: Ryan Bogan <[email protected]>

* Add Changelog entry

Signed-off-by: Ryan Bogan <[email protected]>

* Change entry in changelog to infrastructure

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
(cherry picked from commit a2bb6cc)
martin-gaievski pushed a commit that referenced this pull request Apr 18, 2024
* Add arm64 check when SIMD is disabled

Signed-off-by: Ryan Bogan <[email protected]>

* Add Changelog entry

Signed-off-by: Ryan Bogan <[email protected]>

* Change entry in changelog to infrastructure

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
(cherry picked from commit a2bb6cc)

Co-authored-by: Ryan Bogan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants