Skip to content
New issue

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

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

Already on GitHub? # to your account

Subtraction with underflow in constructor for ImmutableKdTree #138

Closed
claytonwramsey opened this issue Jan 24, 2024 · 4 comments · Fixed by #145
Closed

Subtraction with underflow in constructor for ImmutableKdTree #138

claytonwramsey opened this issue Jan 24, 2024 · 4 comments · Fixed by #145

Comments

@claytonwramsey
Copy link

Hi! I found what seems to be a bug in ImmutableKdTree with a certain set of points for construction. When constructed with a certain set of points, it fails with the following message on debug builds:

thread 'main' panicked at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:379:67:
attempt to subtract with overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/098d4fd74c078b12bfc2e9438a2a04bc18b393bc/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/098d4fd74c078b12bfc2e9438a2a04bc18b393bc/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/098d4fd74c078b12bfc2e9438a2a04bc18b393bc/library/core/src/panicking.rs:144:5
   3: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::update_pivot
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:379:67
   4: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::optimize_stems
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:262:21
   5: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::optimize_stems
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:343:19
   6: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::optimize_stems
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:343:19
   7: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::optimize_stems
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:343:19
   8: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::optimize_stems
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:343:19
   9: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::optimize_stems
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:293:38
  10: kiddo::immutable::float::kdtree::ImmutableKdTree<A,T,_,_>::new_from_slice
             at /home/clayton/.cargo/registry/src/index.crates.io-6f17d22bba15001f/kiddo-4.0.0/src/immutable/float/kdtree.rs:130:35

I was able to capture exactly the set of points that causes the assertion to fail, so this may be of some use for you.

It's a lot of points, but here's a complete repro below:
https://gist.github.com/claytonwramsey/33222355472fef9aa14a002b2b16d58c

@sdd
Copy link
Owner

sdd commented Feb 15, 2024

Hi Clayton, I'm hoping to have time to look into fixing this over the next week or so.

sdd added a commit that referenced this issue Feb 15, 2024
@sdd sdd closed this as completed in #145 Feb 15, 2024
sdd added a commit that referenced this issue Feb 15, 2024
@sdd
Copy link
Owner

sdd commented Feb 15, 2024

I got there in the end Clayton :-)

Will release as part of 4.1.0, sorry for the delay

sdd added a commit that referenced this issue Feb 17, 2024
This was referenced Feb 17, 2024
@sdd
Copy link
Owner

sdd commented Feb 18, 2024

Fix released in 4.2.0 👍🏼

@claytonwramsey
Copy link
Author

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants