saschagrunert / indextree

Arena based tree 🌲 structure by using indices instead of reference counted pointers
MIT License
671 stars 55 forks source link

Why doesn't arena have a way to iterate over nodeids? #105

Open installgentoo opened 5 months ago

installgentoo commented 5 months ago

How am i supposed to filter some ids from arena, for further manipulations? get_node_id searches the entire arena.

Also, how are you supposed to do memoized recursion

    fn pos(&self, date: &Date, node: NodeId, mut arena: TPtr<Arena<Body>>) -> Vec3 {
        let Self { orbit, pos, .. } = self;
        if let Some(p) = pos {
            return *p;
        }

        let a = arena.get_mut();
        let mut n = node;
        let parents = iter::from_fn(|| {
            let p = a[n].parent()?;
            n = p;
            Some(p)
        })
        .collect_vec();

        let arena = arena.clone();
        let parents = parents.into_iter().rev().fold(Vec3((0, 0, 0)), move |acc, n| a[n].get().pos(date, n, arena).sum(acc));

        orbit.pos_at_date(*date).sum(parents)
    }

can't pass arena to to a method, if you're invoking it on something inside arena.

alexmozaidze commented 2 months ago

I'm not sure what exactly your code is trying to do, but you can filter nodes by id using something like this:

arena
    .iter()
    .map(|node| arena.get_node_id(node).expect("conversion infallible for iteration"))
    .filter(|node_id| my_cool_filter_function(node_id));

Arena::get_node_id is actually quite cheap to call; it simply does some bound checks and constructs the NodeId. It shouldn't be much more expensive than indexing into a Vec.

installgentoo commented 2 months ago

I'm not sure what exactly your code is trying to do, but you can filter nodes by id using something like this:

arena
    .iter()
    .map(|node| arena.get_node_id(node).expect("conversion infallible for iteration"))
    .filter(|node_id| my_cool_filter_function(node_id));

Arena::get_node_id is actually quite cheap to call; it simply does some bound checks and constructs the NodeId. It shouldn't be much more expensive than indexing into a Vec.

i do

    pub fn traverse<T>(a: &Arena<T>) -> impl '_ + Iterator<Item = NodeId> {
        (1..=a.count()).map(|i| a.get_node_id_at(i.try_into().valid()).valid())
    }

Seeing as i'm never gonna remove nodes in my specific code.

But my point is, the way to traverse ids should be there already. If you just iter -> map -> getnodeid, that method should be in the library. I, the user, do not know by default that this is cheap, and i should not even concern myself with this, as iterating over the keys/values is a very common case for a container. I initially expected arena.iter() to give you pairs.

alexmozaidze commented 2 months ago

Yeah, it'd be nice to have Arena::iter return Iterator<Item = (&Node<T>, NodeId)>, it'd also be cheap to do, since bound checking isn't needed for we already iterate over valid nodes.