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

First steps to impl ParameterSpace #14

Merged
merged 3 commits into from
Apr 8, 2021
Merged

Conversation

ava57r
Copy link
Collaborator

@ava57r ava57r commented Feb 18, 2021

#5

@ava57r
Copy link
Collaborator Author

ava57r commented Feb 18, 2021

For impl Drop need fix this facebookresearch/faiss#1598

Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. It's true that these bindings are a bit behind on the native Faiss versions, because at some point the project changed to use CMake as the building system and the C API is taking longer to be fully integrated with it. CMake building the C API was recently introduced, but (1) it creates a static library by default instead of a shared object library; (2) it still does not build the GPU part of the C API. I could try looking deeper into this, but my resources on these bindings are limited, and so is my knowledge on CMake.

In the meantime, I can propose a few improvements to what was written here.

src/index/autotune.rs Outdated Show resolved Hide resolved
src/index/autotune.rs Outdated Show resolved Hide resolved
src/index/autotune.rs Outdated Show resolved Hide resolved
troubleshooting.md Outdated Show resolved Hide resolved
@Enet4 Enet4 self-requested a review March 9, 2021 15:10
@Enet4 Enet4 mentioned this pull request Mar 25, 2021
Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@Enet4 Enet4 merged commit cebd46e into Enet4:master Apr 8, 2021
@ava57r ava57r deleted the param-space branch April 9, 2021 12:28
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