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

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

Open
installgentoo opened this issue Apr 4, 2024 · 3 comments
Open

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

installgentoo opened this issue Apr 4, 2024 · 3 comments

Comments

@installgentoo
Copy link

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
Copy link
Contributor

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
Copy link
Author

installgentoo commented Jun 22, 2024

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
Copy link
Contributor

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.

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