From 47ce2b0c74fc8928313adbb3c4d7dad041a1110e Mon Sep 17 00:00:00 2001 From: matlabbe Date: Fri, 7 Aug 2026 20:48:41 -0700 Subject: [PATCH] Fix possible staled flann index (#1742) * Fix possible staled flann index * fixed compatible crc flann index after repair and reload * Added tests * test coverage * testing another possible branch * removed doxygen tag version not opulated yet * updated codedev to ignore test code * Disabled all mac intel ci flaky builds --- .github/workflows/cmake-macos.yml | 14 +- .github/workflows/coverage.yml | 9 + README.md | 1 + codecov.yml | 27 +- corelib/include/rtabmap/core/VWDictionary.h | 5 + corelib/src/Memory.cpp | 24 +- corelib/src/VWDictionary.cpp | 14 +- corelib/test/test_memory.cpp | 263 ++++++++++++++++++++ corelib/test/test_vwdictionary.cpp | 156 ++++++++++++ coverage-report.sh | 7 + doxygen/versions.js | 1 - 11 files changed, 488 insertions(+), 33 deletions(-) diff --git a/.github/workflows/cmake-macos.yml b/.github/workflows/cmake-macos.yml index fbbb3fb8..cbc3780e 100644 --- a/.github/workflows/cmake-macos.yml +++ b/.github/workflows/cmake-macos.yml @@ -23,7 +23,7 @@ jobs: strategy: fail-fast: true matrix: - build_name: [macos-sequoia-apple-silicon, macos-tahoe-intel, macos-tahoe-apple-silicon, macos-tahoe-intel-cv5, macos-tahoe-apple-silicon-cv5] + build_name: [macos-sequoia-apple-silicon, macos-tahoe-apple-silicon, macos-tahoe-apple-silicon-cv5] include: # macos-sequoia-intel temporarily disabled. Had some g2o optimizer issues. #- build_name: macos-sequoia-intel @@ -32,15 +32,15 @@ jobs: - build_name: macos-sequoia-apple-silicon os: macos-15 cv: opencv@4 - - build_name: macos-tahoe-intel - os: macos-26-intel - cv: opencv@4 + # - build_name: macos-tahoe-intel + # os: macos-26-intel + # cv: opencv@4 - build_name: macos-tahoe-apple-silicon os: macos-26 cv: opencv@4 - - build_name: macos-tahoe-intel-cv5 - os: macos-26-intel - cv: opencv + # - build_name: macos-tahoe-intel-cv5 + # os: macos-26-intel + # cv: opencv - build_name: macos-tahoe-apple-silicon-cv5 os: macos-26 cv: opencv diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index c97515de..0249b058 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -100,9 +100,18 @@ jobs: lcov "${LCOV_IGNORE[@]}" --extract lcov.info \ '${{ github.workspace }}/*' \ --output-file lcov.info + # The test sources are the instrument, not the subject: a line in a + # test counts as uncovered only when the test skipped it (a defensive + # cleanup branch, a platform guard), which says nothing about the + # library. They are also near-fully covered by construction, so + # leaving them in inflates the overall number. Excluded here, before + # the upload, so the HTML artifact, the summary below and Codecov all + # report the same figure. Kept in sync with coverage-report.sh. lcov "${LCOV_IGNORE[@]}" --remove lcov.info \ '*/sqlite3/*' \ '*/rtflann/*' \ + '*/corelib/test/*' \ + '*/utilite/test/*' \ --output-file lcov.info lcov --summary lcov.info diff --git a/README.md b/README.md index 9e486d41..75297533 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ rtabmap [![Release][release-image]][releases] [![Downloads][downloads-image]][downloads] +[![codecov](https://codecov.io/gh/introlab/rtabmap/graph/badge.svg?token=mPwvfZMOia)](https://codecov.io/gh/introlab/rtabmap) [![License][license-image]][license] [release-image]: https://img.shields.io/badge/release-0.23.1-green.svg?style=flat diff --git a/codecov.yml b/codecov.yml index f6575f40..c74e0912 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,35 +1,28 @@ # Codecov configuration -- https://docs.codecov.com/docs/codecov-yaml # -# The coverage data itself is produced by .github/workflows/coverage.yml (lcov -# over a Debug build with ENABLE_COVERAGE=ON) and uploaded by -# codecov/codecov-action. This file only controls what Codecov reports back on a -# pull request. -# -# Note: nothing is posted to the PR unless the Codecov GitHub App is installed -# on the repository, and the upload step is skipped entirely when the -# CODECOV_TOKEN secret is unset. +# Coverage data is produced by .github/workflows/coverage.yml (lcov over a Debug +# build with ENABLE_COVERAGE=ON) and uploaded by codecov/codecov-action; this +# file only controls what Codecov reports back on a pull request. Nothing is +# posted unless the Codecov GitHub App has access to the repository. -# Annotate the lines of the diff that no test executed, inline in the -# "Files changed" tab. +# Mark uncovered added lines inline in the "Files changed" tab. github_checks: annotations: true coverage: precision: 2 round: down - range: "60...90" + range: "10...90" # red/green scale: 10% is fully red, 90% fully green status: - # Whole-project coverage, compared against the base commit: catches a slow - # slide down without pinning an absolute number. + # Catch a slow slide down without pinning an absolute number. project: default: target: auto threshold: 1% - # Coverage of the lines this pull request touches -- the number that answers - # "is the new code tested?". Advisory for now: it is reported but does not - # block the merge. Remove "informational" to make it a required check. + # Coverage of the lines this pull request touches. Advisory: reported, but + # does not block the merge -- drop "informational" to make it gate. patch: default: informational: true @@ -37,4 +30,4 @@ coverage: comment: layout: "condensed_header, diff, files" behavior: default - require_changes: true # stay quiet on pull requests that don't move coverage + require_changes: true # stay quiet when coverage doesn't move diff --git a/corelib/include/rtabmap/core/VWDictionary.h b/corelib/include/rtabmap/core/VWDictionary.h index a065446c..ab6b7b8f 100644 --- a/corelib/include/rtabmap/core/VWDictionary.h +++ b/corelib/include/rtabmap/core/VWDictionary.h @@ -336,6 +336,11 @@ public: * @return true if modified, false otherwise */ bool isModified() const; + // Re-index all the words from scratch. The index then contains the words in + // the same order than the one built by update() on a dictionary freshly + // loaded from a database, which is required to serialize it (the serialized + // index is matched against the words in that order when deserialized). + void rebuildIndex(); /** * @brief Serialize the search index to a byte vector diff --git a/corelib/src/Memory.cpp b/corelib/src/Memory.cpp index fd6a150f..51318d38 100644 --- a/corelib/src/Memory.cpp +++ b/corelib/src/Memory.cpp @@ -614,7 +614,12 @@ void Memory::loadDataFromDb(bool postInitClosingEvents) UWARN("%s", msg.c_str()); if(postInitClosingEvents) UEventsManager::post(new RtabmapEventInit(msg)); _memoryChanged = true; // This will force rtabmap to save back the dictionary even if we don't process any new data - _vwd->update(); + // Re-index from scratch instead of adding the words above to the + // index already built: the index would then contain them in a + // different order than the words loaded from the database, which + // makes the index saved on close (see saveFlannIndex()) rejected + // when it is deserialized on next load. + _vwd->rebuildIndex(); } } @@ -714,12 +719,21 @@ void Memory::close(bool databaseSaved, bool postInitClosingEvents, const std::st UINFO("No changes added to database."); if(_dbDriver) { - if(!this->isReadOnly()) { + if(this->isReadOnly()) + { + if(_memoryChanged || _linksChanged || databaseNameChanged) + { + UWARN("Memory has been modified (nodes=%s links=%s name=%s) but the database is read-only, changes are not saved to database.", + _memoryChanged?"true":"false", _linksChanged?"true":"false", databaseNameChanged?"true":"false"); + } + } + else if(databaseSaved) + { saveFlannIndex(postInitClosingEvents); } else if(_memoryChanged || _linksChanged || databaseNameChanged) { - UWARN("Memory has been modified (nodes=%s links=%s name=%s) but the database is read-only, changes are not saved to database.", + UWARN("Memory has been modified (nodes=%s links=%s name=%s) but databaseSaved=false, changes are not saved to database.", _memoryChanged?"true":"false", _linksChanged?"true":"false", databaseNameChanged?"true":"false"); } if(postInitClosingEvents) UEventsManager::post(new RtabmapEventInit(uFormat("Closing database \"%s\"...", _dbDriver->getUrl().c_str()))); @@ -736,8 +750,10 @@ void Memory::close(bool databaseSaved, bool postInitClosingEvents, const std::st { UINFO("Saving memory..."); if(postInitClosingEvents) UEventsManager::post(new RtabmapEventInit("Saving memory...")); - if(!_memoryChanged && _dbDriver) + if(_dbDriver) { + // Must be done before clear(), which clears the dictionary. + // saveFlannIndex() saves only if the dictionary has been modified. saveFlannIndex(postInitClosingEvents); } this->clear(); diff --git a/corelib/src/VWDictionary.cpp b/corelib/src/VWDictionary.cpp index fdd554dc..4a1bee12 100644 --- a/corelib/src/VWDictionary.cpp +++ b/corelib/src/VWDictionary.cpp @@ -118,10 +118,7 @@ void VWDictionary::parseParameters(const ParametersMap & parameters) if(!treeUpdated && byteToFloat!=_byteToFloat && _strategy == kNNFlannKdTree) { UINFO("KDTree: Binary to Float conversion approach has changed, re-initialize kd-tree."); - _dataTree = cv::Mat(); - _notIndexedWords = uKeysSet(_visualWords); - _removedIndexedWords.clear(); - this->update(); + this->rebuildIndex(); } if(incrementalDictionary) @@ -296,6 +293,15 @@ bool VWDictionary::isModified() const return _modified; } +void VWDictionary::rebuildIndex() +{ + UDEBUG("Re-indexing all %ld words...", _visualWords.size()); + _dataTree = cv::Mat(); + _notIndexedWords = uKeysSet(_visualWords); + _removedIndexedWords.clear(); + this->update(); +} + bool VWDictionary::setNNStrategy(NNStrategy strategy) { #if CV_MAJOR_VERSION < 3 diff --git a/corelib/test/test_memory.cpp b/corelib/test/test_memory.cpp index 4f831baa..208a0aae 100644 --- a/corelib/test/test_memory.cpp +++ b/corelib/test/test_memory.cpp @@ -2040,6 +2040,269 @@ TEST(MemoryTest, SetDummyDictionaryIgnoredAfterInit) UFile::erase(dbPath.c_str()); } +namespace { + +// Localization session (Mem/IncrementalMemory=false) with FLANN index +// persistence enabled: the only configuration where Memory::saveFlannIndex() +// writes an index (see Kp/FlannIndexSaved). Incremental FLANN is left on -- +// dictionaryDbParams() disables it -- because that is the path where newly +// indexed words are appended to the existing index instead of rebuilding it. +ParametersMap localizationFlannParams() +{ + ParametersMap params = dictionaryDbParams(); + params[Parameters::kMemIncrementalMemory()] = "false"; + params[Parameters::kKpFlannIndexSaved()] = "true"; + params[Parameters::kKpIncrementalFlann()] = "true"; + return params; +} + +// Whether init() could reuse the index stored in the database. VWDictionary +// only clears its "modified" flag when deserializeIndex() succeeds; an index +// (re)built by update() leaves it set, which is also what makes Memory save it +// back on close. +bool flannIndexReusedFromDb(const Memory & memory) +{ + return !memory.getVWDictionary()->isModified(); +} + +} // namespace + +TEST(MemoryTest, FlannIndexSavedToDatabaseOnlyWhenDatabaseIsSaved) +{ +#ifdef _WIN32 + GTEST_SKIP() << "FlannIndex serialization is not implemented on Windows"; +#else + const std::string dbPath = uniqueDbPath(); + ASSERT_GT(buildDictionaryDb(dbPath), 0); + + // First localization session: nothing stored yet, so the index is built by + // VWDictionary::update(). Discarding the session must not store it either. + { + Memory memory(localizationFlannParams()); + ASSERT_TRUE(memory.init(dbPath)); + ASSERT_FALSE(memory.getVWDictionary()->getVisualWords().empty()); + ASSERT_FALSE(flannIndexReusedFromDb(memory)); + memory.close(false); + } + + // Second session: still no index in the database, this time save it. + { + Memory memory(localizationFlannParams()); + ASSERT_TRUE(memory.init(dbPath)); + EXPECT_FALSE(flannIndexReusedFromDb(memory)) << "close(false) should not have saved a FLANN index"; + memory.close(true); + } + + // Sessions 3, 4 and 5: the index stored by session 2 is reused on every + // re-open, and stays in the database whatever close() is given: + // pass 0, close(false): the discarded session leaves the stored index alone, + // pass 1, close(true) : the dictionary didn't change since it was loaded, so + // the re-save is skipped -- without clearing it, + // pass 2 : final check that the two closes above kept it. + for(int pass = 0; pass < 3; ++pass) + { + SCOPED_TRACE(uFormat("pass %d", pass)); + Memory memory(localizationFlannParams()); + ASSERT_TRUE(memory.init(dbPath)); + const VWDictionary * dictionary = memory.getVWDictionary(); + EXPECT_TRUE(flannIndexReusedFromDb(memory)) << "the stored FLANN index should have been reused"; + EXPECT_EQ((size_t)dictionary->getIndexedWordsCount(), dictionary->getVisualWords().size()); + memory.close(pass == 1); + } + + UFile::erase(dbPath.c_str()); +#endif +} + +TEST(MemoryTest, RepairedDictionaryIndexIsReusableOnNextLoad) +{ +#ifdef _WIN32 + GTEST_SKIP() << "FlannIndex serialization is not implemented on Windows"; +#else + // A repaired dictionary must be re-indexed from scratch, not have its + // recovered words appended to the index already built from the words that + // were still in the database. The next load rebuilds the search data in + // word-id order, so an index holding the recovered words at the end no + // longer matches it and is rejected (see VWDictionary::rebuildIndex()). + const std::string dbPath = uniqueDbPath(); + ASSERT_GT(buildDictionaryDb(dbPath), 0); + + // Drop the lowest word ids while the nodes keep referencing them (what + // happens when rtabmap is killed before it saves the dictionary), so the + // recovered words are exactly the ones that would end up out of order. + { + DBDriver * driver = DBDriver::create(); + ASSERT_NE(driver, nullptr); + ASSERT_TRUE(driver->openConnection(dbPath, false)); + driver->executeNoResult("DELETE FROM Word WHERE id IN (SELECT id FROM Word ORDER BY id ASC LIMIT 2);"); + driver->closeConnection(false); + delete driver; + } + + size_t wordCount = 0; + { + Memory memory(localizationFlannParams()); + ASSERT_TRUE(memory.init(dbPath)); + const VWDictionary * dictionary = memory.getVWDictionary(); + wordCount = dictionary->getVisualWords().size(); + ASSERT_GT(wordCount, 0u); + // Repaired: every word is indexed again, including the recovered ones. + EXPECT_EQ((size_t)dictionary->getIndexedWordsCount(), wordCount); + EXPECT_EQ(dictionary->getNotIndexedWordsCount(), 0u); + // Saves the recovered words back, along with the rebuilt index. + memory.close(true); + } + + // Two re-opens: the first checks the index saved right after the repair is + // reusable, the second that it stays so once a session that merely loaded it + // has closed (nothing changed, so close(true) must leave it in place). + for(int pass = 0; pass < 2; ++pass) + { + SCOPED_TRACE(uFormat("pass %d", pass)); + Memory memory(localizationFlannParams()); + ASSERT_TRUE(memory.init(dbPath)); + const VWDictionary * dictionary = memory.getVWDictionary(); + EXPECT_EQ(dictionary->getVisualWords().size(), wordCount) << "the recovered words should have been saved back"; + EXPECT_TRUE(flannIndexReusedFromDb(memory)) << "the index saved after the repair should still match the stored dictionary"; + EXPECT_EQ((size_t)dictionary->getIndexedWordsCount(), wordCount); + memory.close(pass == 0); + } + + UFile::erase(dbPath.c_str()); +#endif +} + +TEST(MemoryTest, ReadOnlyLocalizationSavesNothingBackAfterRepair) +{ + // Mem/LocalizationReadOnly keeps close() from writing anything back, even + // when the session did change the memory. Repairing the dictionary is such a + // change (it sets _memoryChanged so the recovered words get saved), so a + // read-only session is where close() has to warn and skip the save instead. + const std::string dbPath = uniqueDbPath(); + ASSERT_GT(buildDictionaryDb(dbPath), 0); + + // Remember which words are about to be dropped, to check further down that + // they did not come back. + std::set droppedIds; + { + Memory memory(dictionaryDbParams()); + ASSERT_TRUE(memory.init(dbPath)); + const std::map & words = memory.getVWDictionary()->getVisualWords(); + ASSERT_GE(words.size(), 2u); + std::map::const_iterator iter = words.begin(); + droppedIds.insert(iter->first); + droppedIds.insert((++iter)->first); + memory.close(false); + } + { + DBDriver * driver = DBDriver::create(); + ASSERT_NE(driver, nullptr); + ASSERT_TRUE(driver->openConnection(dbPath, false)); + driver->executeNoResult("DELETE FROM Word WHERE id IN (SELECT id FROM Word ORDER BY id ASC LIMIT 2);"); + driver->closeConnection(false); + delete driver; + } + + ParametersMap params = localizationFlannParams(); + params[Parameters::kMemLocalizationReadOnly()] = "true"; + + size_t wordCount = 0; + { + Memory memory(params); + ASSERT_TRUE(memory.init(dbPath)); + ASSERT_TRUE(memory.isReadOnly()); + wordCount = memory.getVWDictionary()->getVisualWords().size(); + ASSERT_GT(wordCount, 0u); + // The repair happened in memory, and asks for the dictionary to be saved... + EXPECT_TRUE(memory.memoryChanged()); + memory.close(true); // ...which a read-only memory refuses to do. + } + + // The recovered words never reached the database. + { + DBDriver * driver = DBDriver::create(); + ASSERT_NE(driver, nullptr); + ASSERT_TRUE(driver->openConnection(dbPath, false)); + std::list words; + driver->loadWords(droppedIds, words); + EXPECT_TRUE(words.empty()) << "a read-only memory should not have saved the repaired dictionary"; + for(std::list::iterator iter=words.begin(); iter!=words.end(); ++iter) + { + delete *iter; + } + driver->closeConnection(false); + delete driver; + } + + // Neither did the index rebuilt for them, so the next session repairs again. + { + Memory memory(params); + ASSERT_TRUE(memory.init(dbPath)); + EXPECT_EQ(memory.getVWDictionary()->getVisualWords().size(), wordCount); + EXPECT_FALSE(flannIndexReusedFromDb(memory)); + memory.close(false); + } + + UFile::erase(dbPath.c_str()); +} + +TEST(MemoryTest, DiscardedSessionSavesNothingBackWhenMemoryChanged) +{ + // The other side of the same branch: a writable memory that did change, but + // closed with databaseSaved=false. The changes are dropped (with a warning) + // rather than written, so the database keeps the nodes it already had. + const std::string dbPath = uniqueDbPath(); + ASSERT_GT(buildDictionaryDb(dbPath), 0); + + std::set idsBefore; + { + DBDriver * driver = DBDriver::create(); + ASSERT_NE(driver, nullptr); + ASSERT_TRUE(driver->openConnection(dbPath, false)); + driver->getAllNodeIds(idsBefore); + driver->closeConnection(false); + delete driver; + } + ASSERT_FALSE(idsBefore.empty()); + + { + Memory memory(dictionaryDbParams()); + ASSERT_TRUE(memory.init(dbPath)); + ASSERT_FALSE(memory.isReadOnly()); + ASSERT_FALSE(memory.memoryChanged()); + + const int kKeypoints = 3; + cv::Mat image(8, 8, CV_8UC1, cv::Scalar(128)); + SensorData data(image); + std::vector kpts(kKeypoints, cv::KeyPoint(1.f, 1.f, 1.f)); + std::vector pts3(kKeypoints, cv::Point3f(0.f, 0.f, 1.f)); + cv::Mat descriptors = cv::Mat::zeros(kKeypoints, 9, CV_32F); + for(int row = 0; row < kKeypoints; ++row) + { + descriptors.at(row, row) = 1000.0f; + } + data.setFeatures(kpts, pts3, descriptors); + const cv::Mat covariance = cv::Mat::eye(6, 6, CV_64FC1) * 0.01; + ASSERT_TRUE(memory.update(data, Transform(9.0f, 0.0f, 0.0f, 0, 0, 0), covariance)); + ASSERT_TRUE(memory.memoryChanged()); + + memory.close(false); + } + + std::set idsAfter; + { + DBDriver * driver = DBDriver::create(); + ASSERT_NE(driver, nullptr); + ASSERT_TRUE(driver->openConnection(dbPath, false)); + driver->getAllNodeIds(idsAfter); + driver->closeConnection(false); + delete driver; + } + EXPECT_EQ(idsAfter, idsBefore) << "close(false) should not have saved the node added during the session"; + + UFile::erase(dbPath.c_str()); +} + TEST(MemoryTest, ForgetTransfersBasedOnWordCountInWordRegime) { // Branch (1) of Memory::forget() is gated on: diff --git a/corelib/test/test_vwdictionary.cpp b/corelib/test/test_vwdictionary.cpp index 9e67f8e6..c1891ab9 100644 --- a/corelib/test/test_vwdictionary.cpp +++ b/corelib/test/test_vwdictionary.cpp @@ -756,6 +756,162 @@ TEST_F(VWDictionaryTest, SerializeDeserializeIndex) } } +namespace { + +// One-hot descriptor, so every word is far from every other one and the +// checksum over the search data changes as soon as two words are swapped. +cv::Mat oneHotDescriptor(int hotIndex, int dim = 8) +{ + cv::Mat descriptor = cv::Mat::zeros(1, dim, CV_32F); + descriptor.at(0, hotIndex) = 1000.0f; + return descriptor; +} + +} // namespace + +TEST_F(VWDictionaryTest, RebuildIndexReordersIndexToWordIdOrder) +{ + // A dictionary loaded from a database gets its words back in word-id order + // (std::map), and deserializeIndex() rebuilds the search data in that same + // order. So a serialized index is only reusable if it was built in word-id + // order too. + // + // update() with incremental FLANN appends the not-yet-indexed words at the + // end of the existing index, whatever their id. That is what Memory does + // when it repairs a dictionary that is missing words: the repaired words + // usually have ids *lower* than the ones already indexed, so the index ends + // up in an order the next load cannot reproduce. rebuildIndex() re-indexes + // everything from scratch, which restores the word-id order. + dict->setNNStrategy(VWDictionary::kNNFlannKdTree); + ASSERT_TRUE(dict->isIncrementalFlann()) << "The out-of-order index only happens with incremental FLANN"; + + dict->addWord(new VisualWord(1, oneHotDescriptor(0))); + dict->addWord(new VisualWord(3, oneHotDescriptor(2))); + dict->update(); + ASSERT_EQ(dict->getIndexedWordsCount(), 2u); + + // Word 2 is indexed after word 3: index order (1, 3, 2) != id order (1, 2, 3). + dict->addWord(new VisualWord(2, oneHotDescriptor(1))); + dict->update(); + ASSERT_EQ(dict->getIndexedWordsCount(), 3u); + + std::vector staleData = dict->serializeIndex(); +#ifdef _WIN32 + // FlannIndex::serializeIndex() is not implemented on Windows + // (see corelib/src/FlannIndex.cpp), so there is nothing to round-trip. + EXPECT_EQ(staleData.size(), 0u); +#else + ASSERT_GT(staleData.size(), 0u); + + // Simulates the next load: same words, added in id order like DBDriver does. + { + VWDictionary reloaded; + reloaded.setNNStrategy(VWDictionary::kNNFlannKdTree); + reloaded.addWord(new VisualWord(1, oneHotDescriptor(0))); + reloaded.addWord(new VisualWord(2, oneHotDescriptor(1))); + reloaded.addWord(new VisualWord(3, oneHotDescriptor(2))); + EXPECT_FALSE(reloaded.deserializeIndex(staleData)) + << "An index built out of word-id order should be rejected on load"; + } + + // Same words, same content, but re-indexed from scratch. + dict->rebuildIndex(); + EXPECT_EQ(dict->getIndexedWordsCount(), 3u); + EXPECT_EQ(dict->getNotIndexedWordsCount(), 0u); + + std::vector rebuiltData = dict->serializeIndex(); + ASSERT_GT(rebuiltData.size(), 0u); + + { + VWDictionary reloaded; + reloaded.setNNStrategy(VWDictionary::kNNFlannKdTree); + reloaded.addWord(new VisualWord(1, oneHotDescriptor(0))); + reloaded.addWord(new VisualWord(2, oneHotDescriptor(1))); + reloaded.addWord(new VisualWord(3, oneHotDescriptor(2))); + EXPECT_TRUE(reloaded.deserializeIndex(rebuiltData)); + EXPECT_EQ(reloaded.getIndexedWordsCount(), 3u); + // A deserialized index doesn't need to be saved back. + EXPECT_FALSE(reloaded.isModified()); + } +#endif +} + +TEST_F(VWDictionaryTest, RebuildIndexKeepsWordsAndSearchResults) +{ + dict->setNNStrategy(VWDictionary::kNNFlannKdTree); + + cv::Mat descriptors = (cv::Mat_(3, 2) << + 0.0f, 0.0f, + 10.0f, 0.0f, + 0.0f, 100.0f); + std::list wordIds = dict->addNewWords(descriptors, 1); + dict->update(); + ASSERT_EQ(wordIds.size(), 3u); + + cv::Mat query = (cv::Mat_(2, 2) << + 0.5f, 0.5f, // matches the first word + 0.0f, 99.0f); // matches the third word + const std::vector before = dict->findNN(query); + ASSERT_EQ(before.size(), 2u); + ASSERT_EQ(before[0], wordIds.front()); + ASSERT_EQ(before[1], wordIds.back()); + + dict->rebuildIndex(); + + // Re-indexing doesn't touch the words themselves, only the search index. + EXPECT_EQ(dict->getVisualWords().size(), 3u); + EXPECT_EQ(dict->getIndexedWordsCount(), 3u); + EXPECT_EQ(dict->getNotIndexedWordsCount(), 0u); + EXPECT_EQ(dict->findNN(query), before); + + // The index changed, so it has to be saved back (Memory::saveFlannIndex() + // only serializes a modified dictionary). + EXPECT_TRUE(dict->isModified()); +} + +TEST_F(VWDictionaryTest, RebuildIndexOnEmptyDictionaryIsSafe) +{ + dict->setNNStrategy(VWDictionary::kNNFlannKdTree); + + dict->rebuildIndex(); + + EXPECT_EQ(dict->getVisualWords().size(), 0u); + EXPECT_EQ(dict->getIndexedWordsCount(), 0u); + EXPECT_EQ(dict->getNotIndexedWordsCount(), 0u); + EXPECT_TRUE(dict->serializeIndex().empty()); +} + +TEST_F(VWDictionaryTest, ByteToFloatChangeRebuildsIndex) +{ + // parseParameters() re-indexes through rebuildIndex() when the binary to + // float conversion changes, because the descriptors fed to the kd-tree + // change dimension (1 float per byte vs 1 float per bit). + dict->setNNStrategy(VWDictionary::kNNFlannKdTree); + + cv::Mat descriptors = cv::Mat::zeros(3, 4, CV_8U); + for(int row = 0; row < descriptors.rows; ++row) + { + descriptors.at(row, row) = 255; + } + dict->addNewWords(descriptors, 1); + dict->update(); + ASSERT_EQ(dict->getIndexedWordsCount(), 3u); + + ParametersMap params; + params.insert(ParametersPair(Parameters::kKpByteToFloat(), "true")); + dict->parseParameters(params); + + EXPECT_EQ(dict->getVisualWords().size(), 3u); + EXPECT_EQ(dict->getIndexedWordsCount(), 3u); + EXPECT_EQ(dict->getNotIndexedWordsCount(), 0u); + + // The re-indexed dictionary is still searchable, with the smaller + // byte-to-float descriptors this time. + const std::vector matches = dict->findNN(descriptors.row(0)); + ASSERT_EQ(matches.size(), 1u); + EXPECT_NE(matches[0], VWDictionary::ID_INVALID); +} + TEST_F(VWDictionaryTest, IsModified) { EXPECT_TRUE(dict->isModified()); diff --git a/coverage-report.sh b/coverage-report.sh index 8aee4d5f..af1b816b 100755 --- a/coverage-report.sh +++ b/coverage-report.sh @@ -134,9 +134,16 @@ echo "Filtering coverage data (repo sources only)..." lcov "${LCOV_IGNORE[@]}" --extract "$INFO_FILE" \ "${ROOT}/*" \ --output-file "$INFO_FILE" +# The test sources are the instrument, not the subject: a line in a test counts +# as uncovered only when the test skipped it (a defensive cleanup branch, a +# platform guard), which says nothing about the library. They are also +# near-fully covered by construction, so leaving them in inflates the overall +# number. Kept in sync with the same list in .github/workflows/coverage.yml. lcov "${LCOV_IGNORE[@]}" --remove "$INFO_FILE" \ '*/sqlite3/*' \ '*/rtflann/*' \ + '*/corelib/test/*' \ + '*/utilite/test/*' \ --output-file "$INFO_FILE" echo "Generating HTML..." diff --git a/doxygen/versions.js b/doxygen/versions.js index 9c3b074e..64b2573d 100644 --- a/doxygen/versions.js +++ b/doxygen/versions.js @@ -13,5 +13,4 @@ */ window.RTABMAP_DOC_VERSIONS = [ ['latest', 'latest'], - ['0.23.10', '0.23.10'], ];