mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-12 02:13:36 +00:00
Fix C++ reverse_index_factory: missing qtypes crash telemetry, IDMap2 misidentified (#5417)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5417 `reverse_index_factory` in `factory_tools.cpp` has four correctness bugs that affect production telemetry. `TelemetryWrapper::TelemetryWrapper()` calls `reverse_index_factory(this)` with no try/catch; any exception propagates out of the constructor. **Bug 1 — `sq_types.at()` throws for QT_0bit, QT_8bit_uniform, QT_4bit_uniform** `IndexIVFScalarQuantizer` and `IndexScalarQuantizer` constructed with these qtypes pass a key absent from `sq_types` to `std::map::at()`, which throws `std::out_of_range`. Any `TelemetryWrapper<IndexIVFScalarQuantizer>` with one of these qtypes aborts the process. Fix: add all three missing entries. - `QT_0bit → "SQ0"`: accepted by `index_factory` (listed in its name map). - `QT_8bit_uniform → "SQ8u"`, `QT_4bit_uniform → "SQ4u"`: synthetic names, not round-trippable through `index_factory`, but match `contrib/ factory_tools.py` and uniquely identify the type in telemetry. **Bug 2 — IndexIDMap2 returns "IDMap,..." instead of "IDMap2,..."** `IndexIDMap2` inherits `IndexIDMap`. The existing `dynamic_cast<const faiss::IndexIDMap*>` check catches both. Every `TelemetryWrapper<IndexIDMap2>` is logged as `"IDMap,..."` — wrong type. Fix: add `dynamic_cast<const faiss::IndexIDMap2*>` check before the `IndexIDMap` branch, matching the ordering in `contrib/factory_tools.py`. Reviewed By: mnorris11 Differential Revision: D109830472 fbshipit-source-id: 58659b8d54810df89800ad46ef57ffb51c5eaca6
This commit is contained in:
committed by
meta-codesync[bot]
parent
bf53109ec1
commit
7b1d0ca435
@@ -35,11 +35,17 @@ namespace {
|
||||
const std::map<faiss::ScalarQuantizer::QuantizerType, std::string> sq_types = {
|
||||
{faiss::ScalarQuantizer::QT_8bit, "SQ8"},
|
||||
{faiss::ScalarQuantizer::QT_4bit, "SQ4"},
|
||||
// QT_8bit_uniform and QT_4bit_uniform have no round-trippable
|
||||
// index_factory string; the names below are synthetic identifiers used
|
||||
// for telemetry logging only.
|
||||
{faiss::ScalarQuantizer::QT_8bit_uniform, "SQ8u"},
|
||||
{faiss::ScalarQuantizer::QT_4bit_uniform, "SQ4u"},
|
||||
{faiss::ScalarQuantizer::QT_6bit, "SQ6"},
|
||||
{faiss::ScalarQuantizer::QT_fp16, "SQfp16"},
|
||||
{faiss::ScalarQuantizer::QT_bf16, "SQbf16"},
|
||||
{faiss::ScalarQuantizer::QT_8bit_direct_signed, "SQ8_direct_signed"},
|
||||
{faiss::ScalarQuantizer::QT_8bit_direct, "SQ8_direct"},
|
||||
{faiss::ScalarQuantizer::QT_0bit, "SQ0"},
|
||||
{faiss::ScalarQuantizer::QT_1bit_tqmse, "SQtqmse1"},
|
||||
{faiss::ScalarQuantizer::QT_2bit_tqmse, "SQtqmse2"},
|
||||
{faiss::ScalarQuantizer::QT_3bit_tqmse, "SQtqmse3"},
|
||||
@@ -204,6 +210,11 @@ std::string reverse_index_factory(const faiss::Index* index) {
|
||||
const faiss::IndexScalarQuantizer* sq_index =
|
||||
dynamic_cast<const faiss::IndexScalarQuantizer*>(index)) {
|
||||
return sq_types.at(sq_index->sq.qtype);
|
||||
} else if (
|
||||
// IndexIDMap2 inherits IndexIDMap — check subclass first.
|
||||
const faiss::IndexIDMap2* idmap2 =
|
||||
dynamic_cast<const faiss::IndexIDMap2*>(index)) {
|
||||
return std::string("IDMap2,") + reverse_index_factory(idmap2->index);
|
||||
} else if (
|
||||
const faiss::IndexIDMap* idmap =
|
||||
dynamic_cast<const faiss::IndexIDMap*>(index)) {
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
#include <faiss/IndexIDMap.h>
|
||||
#include <faiss/factory_tools.h>
|
||||
#include <faiss/index_factory.h>
|
||||
#include <gtest/gtest.h>
|
||||
@@ -22,6 +23,7 @@ TEST(TestFactoryTools, TestReverseIndexFactory) {
|
||||
"RaBitQ", "IVF8,RaBitQ", "IVF8,SQtqmse8",
|
||||
"EDEN", "EDEN4", "EDEN4BIASED",
|
||||
"IVF8,EDEN", "IVF8,EDEN4", "IVF8,EDEN4BIASED",
|
||||
"IVF8,SQ0",
|
||||
}) {
|
||||
std::unique_ptr<Index> index{index_factory(64, factory)};
|
||||
ASSERT_TRUE(index);
|
||||
@@ -39,4 +41,16 @@ TEST(TestFactoryTools, TestReverseIndexFactory) {
|
||||
}
|
||||
}
|
||||
|
||||
// IndexIDMap2 inherits IndexIDMap; verify the subclass is distinguished.
|
||||
TEST(TestFactoryTools, TestReverseIndexFactoryIDMap) {
|
||||
std::unique_ptr<Index> base{index_factory(64, "Flat")};
|
||||
ASSERT_TRUE(base);
|
||||
|
||||
IndexIDMap idmap(base.get());
|
||||
EXPECT_EQ("IDMap,Flat", reverse_index_factory(&idmap));
|
||||
|
||||
IndexIDMap2 idmap2(base.get());
|
||||
EXPECT_EQ("IDMap2,Flat", reverse_index_factory(&idmap2));
|
||||
}
|
||||
|
||||
} // namespace faiss
|
||||
|
||||
Reference in New Issue
Block a user