Repository navigation
Fix index lookups of the randstrobe with the smallest hash - #661
Merged
Merged
Conversation
bucket_starts[x] is supposed to be the index of the first randstrobe whose bucket (the top bits of its hash) is x or greater. When building it, we pushed a single 0 for the first randstrobe and then, for each new distinct hash, padded the vector up to that hash's bucket with the current position. If the first randstrobe is not in bucket 0 (that is, bucket 0 is empty), the entries from 1 up to the bucket of the first randstrobe were therefore set to the position of the *second* distinct hash. A lookup of the smallest hash then searches a range that starts after it and never finds it. With the default number of bits, there are between two and four randstrobes per bucket on average, so this happens for roughly 2-13% of indexes, including the one for phiX. Fix this by filling bucket_starts with 0 up to and including the bucket of the first randstrobe. Existing .sti files contain the incorrect bucket_starts, so the file format version is bumped to force re-generating them.
marcelm
reviewed
Oct 15, 2026
Comment on lines
+6
to
+8
| * #661: Fix a bug in index construction that, for some references, made the | ||
| randstrobe with the smallest hash value impossible to find. | ||
| Pre-generated index files (`.sti` files) need to be re-generated. |
Collaborator
There was a problem hiding this comment.
TBH, I would probably not write a changelog entry for this. The changelog to me is for users to read about changes relevant to them, and it is very likely that no user has ever been affected by this. If anyone wants to know every change, they can read the Git history.
Collaborator
There was a problem hiding this comment.
I’ll go through the changelog anyway before making the next release. I’ll maybe remove that bullet point then, but let’s leave it in for now.
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
2 participants
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.
When bucket 0 is empty (roughly 2–13% of indexes, including phiX),
make_indexsetbucket_startsfor the buckets up to and including the first randstrobe's bucket to the position of the second distinct hash. As a result, the randstrobe with the smallest hash could never be found.The fix fills those entries with 0 instead. A new test checks that every hash in the phiX index can be looked up; it fails on main. The
.stiformat version is bumped because existing index files contain the incorrect values.Mapping output is unchanged on phiX, chrY, fruit fly, maize and Mo17.