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

Iteration results differ between BasicCOITree and NeonCOITree #16

Open
vsbuffalo opened this issue Dec 17, 2023 · 2 comments
Open

Iteration results differ between BasicCOITree and NeonCOITree #16

vsbuffalo opened this issue Dec 17, 2023 · 2 comments

Comments

@vsbuffalo
Copy link
Contributor

vsbuffalo commented Dec 17, 2023

I was using this library in a project and noticed a subtle potential bug: taking a Vec<Interval<()>> and creating both BasicCOITree and NeonCOITree objects, and then iterating over their intervals with .iter() produces different results. I have created a MRE (mre.rs)

use coitrees::{BasicCOITree, Interval, IntervalTree, NeonCOITree};

fn main() {
    let mut intervals: Vec<Interval<()>> = Vec::new();

    intervals.push(Interval { first: 0, last: 10, metadata: () } );
    intervals.push(Interval { first: 11, last: 14, metadata: () } );

    let basic_trees: BasicCOITree<(), usize> = BasicCOITree::new(&intervals);

    for interval in basic_trees.iter() {
        println!("basic tree interval: {:?}", interval);
    }


    let neon_trees: NeonCOITree<(), usize> = NeonCOITree::new(&intervals);

    for interval in neon_trees.iter() {
        println!("neon tree interval: {:?}", interval);
    }
}

which can be placed in the examples/ directory and run with cargo run --example mre to see the behavior. The output I see is:

basic tree interval: Interval { first: 0, last: 10, metadata: () }
basic tree interval: Interval { first: 11, last: 14, metadata: () }
neon tree interval: Interval { first: -1, last: 11, metadata: () }
neon tree interval: Interval { first: 10, last: 15, metadata: () }

I believe the potential issue could be due to these lines (https://github.com/dcjones/coitrees/blob/main/src/neon.rs#L62-L81). Unfortunately, I do not know enough about targeting these specific architectures to know what is the right way to address this issue and create a pull request. Perhaps these lines are correct, but the iterator for NeonCOITree needs to be adapted to output offset intervals.

cauliyang added a commit to cauliyang/coitrees that referenced this issue Jan 27, 2024
@cauliyang
Copy link
Contributor

@vsbuffalo, Great find! It appears we overlooked adding an offset when converting the coitree to an iterator. This issue is present in both aux and neon. I've taken care of it and fixed the bug with commit d4921e0. You can find this fix in one of my ongoing pull requests at #12.

@vsbuffalo
Copy link
Contributor Author

Awesome, thanks @cauliyang!

cauliyang added a commit to cauliyang/coitrees that referenced this issue Jan 31, 2024
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

2 participants