Document the row / record / TermsAndValues levels and the two senses of "index" - #24
Merged
Merged
Conversation
Four names sit close enough together to read as duplicates of each other, and the pairing of record with TermsAndValues has now been flagged twice as though one of them were redundant. They are four levels: a row is a record plus metadata, identified by a rowNum; a record is the feature data as a concept, whose shape RecordType names; TermsAndValues is the public array pair a record arrives in; LongTermsAndValues is the internal form with terms hashed to long and the uniValue cached alongside. Stating it also explains why the enum is RecordType rather than TermsAndValuesType. README said "A record is a TermsAndValues", which flattens the two levels into each other. It now says a record reaches USSI as one, which is the same instruction for an embedder without the conflation. Docs only.
The audit flagged NearestNeighborSearchIndex as misnamed on the grounds that it holds List<Index> and List<Cache>, so it is the engine rather than an index. That reasoning missed that the word covers two levels and both are right in their place: the library as a whole is an index, which is the sense Uber Similarity Search Index carries, while an Index is one searchable structure inside it. So the facade keeps its name, and the actual confusion - one word, two levels - is stated where it belongs rather than fixed by a rename that would leave a library called an index with nothing named one at the top.
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.
Docs only, two vocabulary levels that keep getting mistaken for duplication.
Rows and records
Four names sit close enough together to read as duplicates, and the record/
TermsAndValuespairing has now been flagged twice as if one of them were redundant. They are four levels of one thing:rowNum, the handle the internals pass around in place of the row.RecordTypenames which shape, and a comparator and a structure pair up by agreeing on one.TermsAndValuesis the public structure a record arrives in:String[] termsandfloat[] values, which between them express every shape depending on population. Terms alone is a sequence, values alone is dense, both is sparse.LongTermsAndValuesis the internal form of the same pair, terms hashed tolongwith the comparator-deriveduniValuecached alongside.Stating it also answers why the enum is
RecordTypeand notTermsAndValuesType.README.mdread "A record is aTermsAndValues", which flattens the two levels into each other. It now reads "A record reaches USSI as aTermsAndValues" — the same instruction for someone embedding the library, without the conflation.Two senses of "index"
This one is a correction to my own audit. I had flagged
NearestNeighborSearchIndexas misnamed because it holdsList<Index>andList<Cache>, making it the engine rather than an index, and proposed renaming it toSimilaritySearchEngine.That reasoning missed that the word covers two levels, and both are right in their place. The library as a whole is an index — that is the sense the project's own name carries in Uber Similarity Search Index — while an
Indexis one searchable structure inside it, a sibling ofCache. Renaming the facade would have left a library called an index with nothing named one at the top, and broken every embedder's entry point to do it.So the name stays and the real confusion, one word covering two levels, is written down instead:
Checked, not guessed
The population rules were read back off
TermsAndValues.isDenseFeature,isSparseFeature, andisSequence. The facade's composition was read off its field declarations:implements AutoCloseableonly, withList<Index> indexesandList<Cache> graduatingCaches.One thing deliberately left out: why we say "active cache" rather than the SPARS paper's "live cache" — the engine owns one active cache plus zero or more graduating caches, and graduating caches are still searched, so "live" would imply the others are dead. That note only makes sense once the paper is cited here, so it rides with the terminology-alignment PR.