Skip to content

Add predecessor and successor functions to RsVec#42

Open
TheDoctor314 wants to merge 8 commits into
Cydhra:masterfrom
TheDoctor314:feature/predecessor-successor
Open

Add predecessor and successor functions to RsVec#42
TheDoctor314 wants to merge 8 commits into
Cydhra:masterfrom
TheDoctor314:feature/predecessor-successor

Conversation

@TheDoctor314
Copy link
Copy Markdown

Resolves #28 .
This is a pretty simple implementation which is taken mostly from the iter module of RsVec.
I'm sure there are some optimization opportunities here. For one thing, I checked that we have the right superblock before checking in the rank-blocks since the rank in blocks is relative to the superblock boundary.

Would appreciate some feedback on this!

@Cydhra
Copy link
Copy Markdown
Owner

Cydhra commented May 31, 2026

I have changed return types to Option<u64>. Using a numeric type is a bad idea since it is unclear what predecessor(0) should return, since the length of the vector is a weirdly non-continuous result, and negative numbers would enforce a signed return type.
Mirroring EliasFanoVec::predecessor, we return None.

Using u64 rather than usize doesn't match the rest of the BitVec but this is subject to change in the future (see #36). To reduce the migration load, new APIs are already following the use of u64 (not to mention that using usize for indices in bitvectors is wrong on 32-bit architectures where bitvectors can easily exceed 2^32 bit.

This broke some of the test cases since they now assume that None is the fallback, but the implementation has not been changed yet. Furthermore, a range of edge cases cause panics or wrong results, I have added new test cases.

In general, adding _unchecked functions that do not return an Option is a valid choice here, but I want the checked versions to return Option. If you think we should add an unchecked version, then you can add them, and add bounds checks to the checked version. The unchecked version may panic on queries like predecessor(0) (it is allowed to return a wrong result, too).

If you don't want to fix those, I will do it later.

@TheDoctor314
Copy link
Copy Markdown
Author

appreciate the feedback on the API practices @Cydhra !
will check your changes and add fixes for the breaking test cases.

@Cydhra Cydhra added the enhancement New feature or request label May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add predecessor1 and predecessor0 (and successor) functions to RsVec

2 participants