Deduplicate the "other sets" in a LocalVocab
#1632
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1632 +/- ##
==========================================
+ Coverage 89.57% 89.58% +0.01%
==========================================
Files 381 381
Lines 36792 36818 +26
Branches 4170 4173 +3
==========================================
+ Hits 32955 32983 +28
+ Misses 2525 2522 -3
- Partials 1312 1313 +1 ☔ View full report in Codecov by Sentry. |
hannahbast
changed the title
LocalVocabLocalVocab
There was a problem hiding this comment.
@RobinTF Thanks a lot for this PR. I looked at it and discussed it with @joka921 . It is fine, with the following addition:
For operations with a non-empty local vocab, add the size_ and otherWordSets_.size() to the details of the runtime information. For example in the form local-vocab-size: {size_} [{otherWordSets_.size()}]. That way, it will not go unnoticed when the local vocab becomes large or complex.
| runtimeInfo().addDetail("num-local-vocabs", numLocalVocabs); | ||
| size_t vocabSize = result.localVocab().size(); | ||
| if (vocabSize > 1) { | ||
| runtimeInfo().addDetail("local-vocab-size", vocabSize); |
There was a problem hiding this comment.
We also want the number of non-local-vocabs that are merged together in the materialized local vocab here (if it is greater than >1), solocal-vocab-num-sets` would be a nice name here.
| absl::StrCat(vocabStats.nonEmptyVocabs_, " / ", | ||
| vocabStats.totalVocabs_, | ||
| ", Ø = ", vocabStats.avgSize(), | ||
| ", max = ", vocabStats.maxSize_)); |
There was a problem hiding this comment.
Also the number of contained sets (similar statistics, max/avg) would be good to know here.
Conformance check passed ✅No test result changes. |
|
Since #1524, lazy joins can produce
LocalVocabs with many duplicate "other sets". These duplicates cannot be easily detected because they are stored in astd::vector. They are now stored as aabsl:flat_hash_setinstead. This may come with a small performance penalty when the size of this set becomes large. To be able to detect this, add the size of the set to the details of the runtime information.Based on the suggestion from #1524 (comment)