Name keys as terms and signatures, and drop "active" from the threshold - #27
Merged
Merged
Conversation
A key is the collective name for what an inverted list can be keyed by: a term or a signature. The record vocabulary section now says so, and says that every record holds terms whatever its shape, since a sequence's ordered items live in the same terms array a sparse record's do. Sequence prose keeps saying "element" where order is the point, which reads better than "term" there, but it is a reading aid rather than a third kind of thing and the section says so. The code has no element concept: the only element identifiers in the repo are BoundedSizeMaxHeap's, the ordinary Java collection sense. DESIGN.md also called the threshold the "active similarity threshold", which matches nothing: the code calls it minSimilarity, and this document gives "active" a different and specific meaning, the one cache accepting writes while the graduating ones only serve reads. Two of the three occurrences now name minSimilarity; the third sits on a line the prefix-filter PR rewrites, so it is left to that PR to keep the two line-disjoint. Not adopting the paper's terms here, because in each case the code turned out to name the thing: comparison threshold is minSimilarity, popularity cutoff is maxNumRowsPerTerm, and record frequency is what max_fraction_ids_per_term is stated against. "Alphabet" needed no work, already in use. Docs only. 547 tests pass, 3 pre-existing OpenBLAS aborts.
drametwally
force-pushed
the
threshold-and-element-wording
branch
from
September 15, 2026 03:48
139f3c3 to
bbe0e71
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of the two terminology PRs, a sibling of #26 off
main. The two touch disjoint lines and can merge in either order.Revised after review. The first version defined term, element, and key as three parallel words. That was wrong: key is the collective name for what an inverted list can be keyed by, which is a term or a signature, and the code has no element concept at all. The only
elementidentifiers in the repo areBoundedSizeMaxHeap's, the ordinary Java collection sense; every sequence-domain use is prose in comments. A sequence's ordered items live in the sametermsarray a sparse record's do, so to the code they are terms.So the section now says two things instead of three: every record holds terms whatever its shape, and a key is a term or a signature and therefore not always something the record itself holds. It also notes that sequence prose says "element" where order is the point, as a reading aid rather than a third kind of thing.
"active similarity threshold" matches nothing
Three occurrences, wrong twice over. The code calls this
minSimilarity, and separately this document gives active a specific and different meaning: the one cache accepting writes, as opposed to the graduating caches, which are still searched and still serve results. Reusing it for a threshold spends a word that already has a job.Two occurrences now name
minSimilarity. The third is on a line #26 rewrites, so I left it there rather than stack the branches — after the earlier detached-base incident I am keeping these as siblings. A reviewer grepping this branch will still find that one occurrence; it goes away with #26.The paper-term item dissolved
Every term I had queued turned out to be one the code already names, so the authority rule — code wins for anything the code names, paper wins only for what it doesn't — rejected all of them:
minSimilarity, 224 occurrencesmaxNumRowsPerTerm, pluspopularTerms/popularTermDiscardScopemax_fraction_ids_per_termis already stated againstVerification
rg -U --multiline-dotall.