From 17fda2692261afd9c536803dddf0ad2202f2c18f Mon Sep 17 00:00:00 2001 From: "xiaojun.lin" Date: Mon, 25 Nov 2019 16:17:16 +0800 Subject: [PATCH 1/4] fix issue-509 --- CHANGELOG.md | 1 + core/src/wrapper/ConfAdapter.cpp | 30 +++++++++++++++++++----------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53e427dcb9..8012630e0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Please mark all change in change log and use the ticket from JIRA. - \#440 - Server cannot startup with gpu_resource_config.enable=false in GPU version - \#458 - Index data is not compatible between 0.5 and 0.6 - \#486 - gpu no usage during index building +- \#509 - IVF_PQ index build trapped into dead loop caused by invalid params ## Feature - \#12 - Pure CPU version for Milvus diff --git a/core/src/wrapper/ConfAdapter.cpp b/core/src/wrapper/ConfAdapter.cpp index aa4b3c12b8..cf9f1a0474 100644 --- a/core/src/wrapper/ConfAdapter.cpp +++ b/core/src/wrapper/ConfAdapter.cpp @@ -22,6 +22,7 @@ #include #include +#include // TODO(lxj): add conf checker @@ -121,6 +122,13 @@ IVFSQConfAdapter::Match(const TempMetaConf& metaconf) { return conf; } +#define MatchSubQuantizer(c) \ + if (!(conf->d % c)) { \ + WRAPPER_LOG_DEBUG << "PQ m = " << conf->d / c; \ + conf->m = conf->d / c; \ + return conf; \ + } + knowhere::Config IVFPQConfAdapter::Match(const TempMetaConf& metaconf) { auto conf = std::make_shared(); @@ -129,18 +137,18 @@ IVFPQConfAdapter::Match(const TempMetaConf& metaconf) { conf->metric_type = metaconf.metric_type; conf->gpu_id = metaconf.gpu_id; conf->nbits = 8; - - if (!(conf->d % 4)) - conf->m = conf->d / 4; // compression radio = 16 - else if (!(conf->d % 2)) - conf->m = conf->d / 2; // compression radio = 8 - else if (!(conf->d % 3)) - conf->m = conf->d / 3; // compression radio = 12 - else - conf->m = conf->d; // same as SQ8, compression radio = 4 - MatchBase(conf); - return conf; + + /* + * Faiss 1.6 + * Only 1, 2, 3, 4, 6, 8, 10, 12, 16, 20, 24, 28, 32 dims per sub-quantizer are currently supporte with + * no precomputed codes. Precomputed codes supports any number of dimensions, but will involve memory overheads. + */ + static std::vector support_sub_quantizer{32, 28, 24, 20, 16, 12, 10, 8, 6, 4, 3, 2, 1}; + for (const auto& c : support_sub_quantizer) { + // compression radio = dim / c * 4 + MatchSubQuantizer(c) + } } knowhere::Config From 76aaef7bd79780f7c0d30b6c0a6d04b6669473f9 Mon Sep 17 00:00:00 2001 From: "xiaojun.lin" Date: Mon, 25 Nov 2019 16:51:11 +0800 Subject: [PATCH 2/4] review change --- core/src/wrapper/ConfAdapter.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/core/src/wrapper/ConfAdapter.cpp b/core/src/wrapper/ConfAdapter.cpp index cf9f1a0474..501ada41d6 100644 --- a/core/src/wrapper/ConfAdapter.cpp +++ b/core/src/wrapper/ConfAdapter.cpp @@ -122,13 +122,6 @@ IVFSQConfAdapter::Match(const TempMetaConf& metaconf) { return conf; } -#define MatchSubQuantizer(c) \ - if (!(conf->d % c)) { \ - WRAPPER_LOG_DEBUG << "PQ m = " << conf->d / c; \ - conf->m = conf->d / c; \ - return conf; \ - } - knowhere::Config IVFPQConfAdapter::Match(const TempMetaConf& metaconf) { auto conf = std::make_shared(); @@ -146,8 +139,11 @@ IVFPQConfAdapter::Match(const TempMetaConf& metaconf) { */ static std::vector support_sub_quantizer{32, 28, 24, 20, 16, 12, 10, 8, 6, 4, 3, 2, 1}; for (const auto& c : support_sub_quantizer) { - // compression radio = dim / c * 4 - MatchSubQuantizer(c) + if (!(conf->d % c)) { + conf->m = conf->d / c; + WRAPPER_LOG_DEBUG << "PQ m = " << conf->d / c << ", compression radio = " << conf->d / c * 4; + return conf; + } } } From f25992f596c239a3c1e8b9a9748cb7aa33dc5120 Mon Sep 17 00:00:00 2001 From: "xiaojun.lin" Date: Mon, 25 Nov 2019 20:01:06 +0800 Subject: [PATCH 3/4] update --- core/src/wrapper/ConfAdapter.cpp | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/core/src/wrapper/ConfAdapter.cpp b/core/src/wrapper/ConfAdapter.cpp index 501ada41d6..f362808940 100644 --- a/core/src/wrapper/ConfAdapter.cpp +++ b/core/src/wrapper/ConfAdapter.cpp @@ -137,14 +137,30 @@ IVFPQConfAdapter::Match(const TempMetaConf& metaconf) { * Only 1, 2, 3, 4, 6, 8, 10, 12, 16, 20, 24, 28, 32 dims per sub-quantizer are currently supporte with * no precomputed codes. Precomputed codes supports any number of dimensions, but will involve memory overheads. */ - static std::vector support_sub_quantizer{32, 28, 24, 20, 16, 12, 10, 8, 6, 4, 3, 2, 1}; - for (const auto& c : support_sub_quantizer) { - if (!(conf->d % c)) { - conf->m = conf->d / c; - WRAPPER_LOG_DEBUG << "PQ m = " << conf->d / c << ", compression radio = " << conf->d / c * 4; - return conf; + static std::vector support_dim_per_subquantizer{32, 28, 24, 20, 16, 12, 10, 8, 6, 4, 3, 2, 1}; + static std::vector support_subquantizer{96, 64, 56, 48, 40, 32, 28, 24, 20, 16, 12, 8, 4, 3, 2, 1}; + std::vector resset; + for (const auto& dimperquantizer : support_dim_per_subquantizer) { + if (!(conf->d % dimperquantizer)) { + auto subquantzier_num = conf->d / dimperquantizer; + auto finder = std::find(support_subquantizer.begin(), support_subquantizer.end(), subquantzier_num); + if (finder != support_subquantizer.end()) { + resset.push_back(subquantzier_num); + } } } + + if (resset.empty()) { + // todo(linxj): throw exception here. + return nullptr; + } + + static int64_t compression_level = 1; // 1:low, 2:high + if (compression_level == 1) { + conf->m = resset[int(resset.size()/2)]; + WRAPPER_LOG_DEBUG << "PQ m = " << conf->m << ", compression radio = " << conf->d / conf->m * 4; + } + return conf; } knowhere::Config From 1f342f950b55e2ddb48ab2d2e4a96647a63630ee Mon Sep 17 00:00:00 2001 From: Tinkerrr Date: Mon, 25 Nov 2019 22:16:18 +0800 Subject: [PATCH 4/4] format code --- core/src/wrapper/ConfAdapter.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/wrapper/ConfAdapter.cpp b/core/src/wrapper/ConfAdapter.cpp index f362808940..d49747d8f4 100644 --- a/core/src/wrapper/ConfAdapter.cpp +++ b/core/src/wrapper/ConfAdapter.cpp @@ -154,11 +154,10 @@ IVFPQConfAdapter::Match(const TempMetaConf& metaconf) { // todo(linxj): throw exception here. return nullptr; } - - static int64_t compression_level = 1; // 1:low, 2:high + static int64_t compression_level = 1; // 1:low, 2:high if (compression_level == 1) { - conf->m = resset[int(resset.size()/2)]; - WRAPPER_LOG_DEBUG << "PQ m = " << conf->m << ", compression radio = " << conf->d / conf->m * 4; + conf->m = resset[int(resset.size() / 2)]; + WRAPPER_LOG_DEBUG << "PQ m = " << conf->m << ", compression radio = " << conf->d / conf->m * 4; } return conf; }