Update faiss version in both Dockerfiles and any required API updates#251
Update faiss version in both Dockerfiles and any required API updates#251
Conversation
|
Target CPP Coverage: 64.1637% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
|
@s-gobriel, code changes look good to me. You can mark as |
|
Target CPP Coverage: 64.1609% Target Python Coverage: 98.02% |
keirafadams
left a comment
There was a problem hiding this comment.
Nothing leaping out at me, only one comment is some magic numbers we might want to take care of, but I dont think that should block merging given this was hanging around a bit on accident.
| if (metric == L2) { | ||
| _index = new faiss::IndexHNSWFlat(dim, hnsw_M, faiss::METRIC_L2); | ||
| ((faiss::IndexHNSWFlat *)_index)->hnsw.efConstruction = 96; | ||
| } else if (metric == IP) { |
There was a problem hiding this comment.
Magic Numbers, not going to block review but do we want to flag for moving to named var/constant?
Closes #222
Updating Faiss from v1.7.4 to v1.9.0
Add any necessary API updates