Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Radix root model not working with small branching factor #3

Open
marcelmaltry opened this issue Aug 12, 2020 · 2 comments
Open

Radix root model not working with small branching factor #3

marcelmaltry opened this issue Aug 12, 2020 · 2 comments

Comments

@marcelmaltry
Copy link

Hi, I recently tried to build a model with a radix layer and noticed that radix models do not work with small branching factors (1, 2, 3), e.g.

cargo run --release -- uniform_dense_200M_uint64 sosd_rmi radix,linear 1

I always get an error similar to this:

thread '<unnamed>' panicked at 'Top model gave an index of 2954937499648 which is out of bounds of 2. Subset range: 1 to 200000000', /private/tmp/rust-20200803-28615-1977jkb/rustc-1.45.2-src/src/libstd/macros.rs:16:9

I suspect that the issue stems from models::utils::num_bits. I guess you are computing the amount of bits that are needed to represent the largest index (while loop) and then substract 1 to ensure that the radix model always predicts an index smaller than the branching_factor. However, your implementation appears to be off by 1, i.e. the additional nbits -= 1 is not needed.

@ImmanuelHaffner
Copy link

ImmanuelHaffner commented Aug 13, 2020

I suppose a simpler (and faster) solution to computing the number of bits required can be found here. While this gives you floor(log2(x)), we can use it to implement ceil(log2(x)) as floor(log2(x - 1)) + 1. The code for your case of 64 bit integers would then be

64 - (largest_target - 1).leading_zeros()

@RyanMarcus
Copy link
Contributor

Hm, that's a strange one. I have no idea what I was thinking when I wrote that code!

Any two-layer RMI with only a single leaf node is the same as a one-layer RMI, so the code should catch that and warn the user as well.

RyanMarcus added a commit that referenced this issue Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants