From d76a0072339a18ffe6e586ef7656b066095a73c5 Mon Sep 17 00:00:00 2001 From: imrn99 <95699343+imrn99@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:07:18 +0100 Subject: [PATCH 1/5] add new variants --- honeycomb-core/src/cmap/dim2/orbits.rs | 30 ++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/honeycomb-core/src/cmap/dim2/orbits.rs b/honeycomb-core/src/cmap/dim2/orbits.rs index e50c5aae6..b2bb8230e 100644 --- a/honeycomb-core/src/cmap/dim2/orbits.rs +++ b/honeycomb-core/src/cmap/dim2/orbits.rs @@ -20,10 +20,14 @@ use std::collections::{BTreeSet, VecDeque}; pub enum OrbitPolicy { /// 0-cell orbit. Vertex, + /// 0-cell orbit, without using beta 0. This requires the cell to be complete / closed. + VertexLinear, /// 1-cell orbit. Edge, /// 2-cell orbit. Face, + /// 2-cell orbit, without using beta 0. This requires the cell to be complete / closed. + FaceLinear, /// Ordered array of beta functions that define the orbit. Custom(&'static [u8]), } @@ -125,7 +129,6 @@ impl<'a, T: CoordsFloat> Iterator for Orbit2<'a, T> { match self.orbit_policy { OrbitPolicy::Vertex => { // THIS CODE IS ONLY VALID IN 2D - let image1 = self.map_handle.beta::<1>(self.map_handle.beta::<2>(d)); if self.marked.insert(image1) { // if true, we did not see this dart yet @@ -139,6 +142,15 @@ impl<'a, T: CoordsFloat> Iterator for Orbit2<'a, T> { self.pending.push_back(image2); } } + OrbitPolicy::VertexLinear => { + // THIS CODE IS ONLY VALID IN 2D + let image = self.map_handle.beta::<1>(self.map_handle.beta::<2>(d)); + if self.marked.insert(image) { + // if true, we did not see this dart yet + // i.e. we need to visit it later + self.pending.push_back(image); + } + } OrbitPolicy::Edge => { // THIS CODE IS ONLY VALID IN 2D let image = self.map_handle.beta::<2>(d); @@ -150,7 +162,21 @@ impl<'a, T: CoordsFloat> Iterator for Orbit2<'a, T> { } OrbitPolicy::Face => { // THIS CODE IS ONLY VALID IN 2D - // WE ASSUME THAT THE FACE IS COMPLETE + let image1 = self.map_handle.beta::<1>(d); + if self.marked.insert(image1) { + // if true, we did not see this dart yet + // i.e. we need to visit it later + self.pending.push_back(image1); + } + let image2 = self.map_handle.beta::<0>(d); + if self.marked.insert(image2) { + // if true, we did not see this dart yet + // i.e. we need to visit it later + self.pending.push_back(image2); + } + } + OrbitPolicy::FaceLinear => { + // THIS CODE IS ONLY VALID IN 2D let image = self.map_handle.beta::<1>(d); if self.marked.insert(image) { // if true, we did not see this dart yet From 944b697ed3bc7cd347977f381629353db7b1fdee Mon Sep 17 00:00:00 2001 From: imrn99 <95699343+imrn99@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:12:55 +0100 Subject: [PATCH 2/5] fix match arms --- honeycomb-core/src/attributes/manager.rs | 64 ++++++++++++++++-------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/honeycomb-core/src/attributes/manager.rs b/honeycomb-core/src/attributes/manager.rs index 21a70e91a..1b44a9c06 100644 --- a/honeycomb-core/src/attributes/manager.rs +++ b/honeycomb-core/src/attributes/manager.rs @@ -21,9 +21,11 @@ use std::{any::TypeId, collections::HashMap}; macro_rules! get_storage { ($slf: ident, $id: ident) => { let probably_storage = match A::BIND_POLICY { - OrbitPolicy::Vertex => $slf.vertices.get(&TypeId::of::()), + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { + $slf.vertices.get(&TypeId::of::()) + } OrbitPolicy::Edge => $slf.edges.get(&TypeId::of::()), - OrbitPolicy::Face => $slf.faces.get(&TypeId::of::()), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => $slf.faces.get(&TypeId::of::()), OrbitPolicy::Custom(_) => $slf.others.get(&TypeId::of::()), }; let $id = probably_storage @@ -35,9 +37,11 @@ macro_rules! get_storage { macro_rules! get_storage_mut { ($slf: ident, $id: ident) => { let probably_storage = match A::BIND_POLICY { - OrbitPolicy::Vertex => $slf.vertices.get_mut(&TypeId::of::()), + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { + $slf.vertices.get_mut(&TypeId::of::()) + } OrbitPolicy::Edge => $slf.edges.get_mut(&TypeId::of::()), - OrbitPolicy::Face => $slf.faces.get_mut(&TypeId::of::()), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => $slf.faces.get_mut(&TypeId::of::()), OrbitPolicy::Custom(_) => $slf.others.get_mut(&TypeId::of::()), }; let $id = probably_storage @@ -162,9 +166,13 @@ impl AttrStorageManager { let typeid = TypeId::of::(); let new_storage = ::StorageType::new(size); if match A::BIND_POLICY { - OrbitPolicy::Vertex => self.vertices.insert(typeid, Box::new(new_storage)), + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { + self.vertices.insert(typeid, Box::new(new_storage)) + } OrbitPolicy::Edge => self.edges.insert(typeid, Box::new(new_storage)), - OrbitPolicy::Face => self.faces.insert(typeid, Box::new(new_storage)), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { + self.faces.insert(typeid, Box::new(new_storage)) + } OrbitPolicy::Custom(_) => self.others.insert(typeid, Box::new(new_storage)), } .is_some() @@ -212,9 +220,9 @@ impl AttrStorageManager { #[must_use = "unused getter result - please remove this method call"] pub fn get_storage(&self) -> Option<&::StorageType> { let probably_storage = match A::BIND_POLICY { - OrbitPolicy::Vertex => &self.vertices[&TypeId::of::()], + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => &self.vertices[&TypeId::of::()], OrbitPolicy::Edge => &self.edges[&TypeId::of::()], - OrbitPolicy::Face => &self.faces[&TypeId::of::()], + OrbitPolicy::Face | OrbitPolicy::FaceLinear => &self.faces[&TypeId::of::()], OrbitPolicy::Custom(_) => &self.others[&TypeId::of::()], }; probably_storage.downcast_ref::<::StorageType>() @@ -231,9 +239,11 @@ impl AttrStorageManager { pub fn remove_storage(&mut self) { // we could return it ? let _ = match A::BIND_POLICY { - OrbitPolicy::Vertex => &self.vertices.remove(&TypeId::of::()), + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { + &self.vertices.remove(&TypeId::of::()) + } OrbitPolicy::Edge => &self.edges.remove(&TypeId::of::()), - OrbitPolicy::Face => &self.faces.remove(&TypeId::of::()), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => &self.faces.remove(&TypeId::of::()), OrbitPolicy::Custom(_) => &self.others.remove(&TypeId::of::()), }; } @@ -256,9 +266,13 @@ impl AttrStorageManager { id_in_rhs: DartIdType, ) { match orbit_policy { - OrbitPolicy::Vertex => self.force_merge_vertex_attributes(id_out, id_in_lhs, id_in_rhs), + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { + self.force_merge_vertex_attributes(id_out, id_in_lhs, id_in_rhs) + } OrbitPolicy::Edge => self.force_merge_edge_attributes(id_out, id_in_lhs, id_in_rhs), - OrbitPolicy::Face => self.force_merge_face_attributes(id_out, id_in_lhs, id_in_rhs), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { + self.force_merge_face_attributes(id_out, id_in_lhs, id_in_rhs) + } OrbitPolicy::Custom(_) => unimplemented!(), } } @@ -335,11 +349,13 @@ impl AttrStorageManager { id_in_rhs: DartIdType, ) -> StmResult<()> { match orbit_policy { - OrbitPolicy::Vertex => { + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { self.merge_vertex_attributes(trans, id_out, id_in_lhs, id_in_rhs) } OrbitPolicy::Edge => self.merge_edge_attributes(trans, id_out, id_in_lhs, id_in_rhs), - OrbitPolicy::Face => self.merge_face_attributes(trans, id_out, id_in_lhs, id_in_rhs), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { + self.merge_face_attributes(trans, id_out, id_in_lhs, id_in_rhs) + } OrbitPolicy::Custom(_) => unimplemented!(), } } @@ -448,13 +464,13 @@ impl AttrStorageManager { id_in_rhs: DartIdType, ) -> CMapResult<()> { match orbit_policy { - OrbitPolicy::Vertex => { + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { self.try_merge_vertex_attributes(trans, id_out, id_in_lhs, id_in_rhs) } OrbitPolicy::Edge => { self.try_merge_edge_attributes(trans, id_out, id_in_lhs, id_in_rhs) } - OrbitPolicy::Face => { + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { self.try_merge_face_attributes(trans, id_out, id_in_lhs, id_in_rhs) } OrbitPolicy::Custom(_) => unimplemented!(), @@ -637,11 +653,13 @@ impl AttrStorageManager { id_in: DartIdType, ) { match orbit_policy { - OrbitPolicy::Vertex => { + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { self.force_split_vertex_attributes(id_out_lhs, id_out_rhs, id_in); } OrbitPolicy::Edge => self.force_split_edge_attributes(id_out_lhs, id_out_rhs, id_in), - OrbitPolicy::Face => self.force_split_face_attributes(id_out_lhs, id_out_rhs, id_in), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { + self.force_split_face_attributes(id_out_lhs, id_out_rhs, id_in) + } OrbitPolicy::Custom(_) => unimplemented!(), } } @@ -721,11 +739,13 @@ impl AttrStorageManager { id_in: DartIdType, ) -> StmResult<()> { match orbit_policy { - OrbitPolicy::Vertex => { + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { self.split_vertex_attributes(trans, id_out_lhs, id_out_rhs, id_in) } OrbitPolicy::Edge => self.split_edge_attributes(trans, id_out_lhs, id_out_rhs, id_in), - OrbitPolicy::Face => self.split_face_attributes(trans, id_out_lhs, id_out_rhs, id_in), + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { + self.split_face_attributes(trans, id_out_lhs, id_out_rhs, id_in) + } OrbitPolicy::Custom(_) => unimplemented!(), } } @@ -835,13 +855,13 @@ impl AttrStorageManager { id_in: DartIdType, ) -> CMapResult<()> { match orbit_policy { - OrbitPolicy::Vertex => { + OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { self.try_split_vertex_attributes(trans, id_out_lhs, id_out_rhs, id_in) } OrbitPolicy::Edge => { self.try_split_edge_attributes(trans, id_out_lhs, id_out_rhs, id_in) } - OrbitPolicy::Face => { + OrbitPolicy::Face | OrbitPolicy::FaceLinear => { self.try_split_face_attributes(trans, id_out_lhs, id_out_rhs, id_in) } OrbitPolicy::Custom(_) => unimplemented!(), From fe408d0e34066afe8d26b317b6aff3c4171ba3c5 Mon Sep 17 00:00:00 2001 From: imrn99 <95699343+imrn99@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:15:11 +0100 Subject: [PATCH 3/5] fix tests --- honeycomb-core/src/cmap/builder/grid/tests.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/honeycomb-core/src/cmap/builder/grid/tests.rs b/honeycomb-core/src/cmap/builder/grid/tests.rs index 3f278220c..5fc9baad2 100644 --- a/honeycomb-core/src/cmap/builder/grid/tests.rs +++ b/honeycomb-core/src/cmap/builder/grid/tests.rs @@ -112,11 +112,12 @@ fn square_cmap2_correctness() { assert_eq!(cmap.face_id(3), 1); assert_eq!(cmap.face_id(4), 1); + // i-cell uses beta 0 to ensure correctness, so the iterator is BFS-like let mut face = cmap.i_cell::<2>(1); assert_eq!(face.next(), Some(1)); - assert_eq!(face.next(), Some(2)); - assert_eq!(face.next(), Some(3)); - assert_eq!(face.next(), Some(4)); + assert_eq!(face.next(), Some(2)); // b1 + assert_eq!(face.next(), Some(4)); // b0 + assert_eq!(face.next(), Some(3)); // b1b1 assert_eq!(face.next(), None); assert_eq!(cmap.beta::<1>(1), 2); @@ -138,8 +139,8 @@ fn square_cmap2_correctness() { let mut face = cmap.i_cell::<2>(5); assert_eq!(face.next(), Some(5)); assert_eq!(face.next(), Some(6)); - assert_eq!(face.next(), Some(7)); assert_eq!(face.next(), Some(8)); + assert_eq!(face.next(), Some(7)); assert_eq!(face.next(), None); assert_eq!(cmap.beta::<1>(5), 6); @@ -161,8 +162,8 @@ fn square_cmap2_correctness() { let mut face = cmap.i_cell::<2>(9); assert_eq!(face.next(), Some(9)); assert_eq!(face.next(), Some(10)); - assert_eq!(face.next(), Some(11)); assert_eq!(face.next(), Some(12)); + assert_eq!(face.next(), Some(11)); assert_eq!(face.next(), None); assert_eq!(cmap.beta::<1>(9), 10); @@ -184,8 +185,8 @@ fn square_cmap2_correctness() { let mut face = cmap.i_cell::<2>(13); assert_eq!(face.next(), Some(13)); assert_eq!(face.next(), Some(14)); - assert_eq!(face.next(), Some(15)); assert_eq!(face.next(), Some(16)); + assert_eq!(face.next(), Some(15)); assert_eq!(face.next(), None); assert_eq!(cmap.beta::<1>(13), 14); From 2651b69d0067e037ffe0d2f1d0cfc48a71852f50 Mon Sep 17 00:00:00 2001 From: imrn99 <95699343+imrn99@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:21:07 +0100 Subject: [PATCH 4/5] fix clippy --- honeycomb-core/src/attributes/manager.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/honeycomb-core/src/attributes/manager.rs b/honeycomb-core/src/attributes/manager.rs index 1b44a9c06..d8691beab 100644 --- a/honeycomb-core/src/attributes/manager.rs +++ b/honeycomb-core/src/attributes/manager.rs @@ -267,11 +267,11 @@ impl AttrStorageManager { ) { match orbit_policy { OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => { - self.force_merge_vertex_attributes(id_out, id_in_lhs, id_in_rhs) + self.force_merge_vertex_attributes(id_out, id_in_lhs, id_in_rhs); } OrbitPolicy::Edge => self.force_merge_edge_attributes(id_out, id_in_lhs, id_in_rhs), OrbitPolicy::Face | OrbitPolicy::FaceLinear => { - self.force_merge_face_attributes(id_out, id_in_lhs, id_in_rhs) + self.force_merge_face_attributes(id_out, id_in_lhs, id_in_rhs); } OrbitPolicy::Custom(_) => unimplemented!(), } @@ -658,7 +658,7 @@ impl AttrStorageManager { } OrbitPolicy::Edge => self.force_split_edge_attributes(id_out_lhs, id_out_rhs, id_in), OrbitPolicy::Face | OrbitPolicy::FaceLinear => { - self.force_split_face_attributes(id_out_lhs, id_out_rhs, id_in) + self.force_split_face_attributes(id_out_lhs, id_out_rhs, id_in); } OrbitPolicy::Custom(_) => unimplemented!(), } From 7de7dccbb18e94e064fac88e767d014a2a2a74fa Mon Sep 17 00:00:00 2001 From: imrn99 <95699343+imrn99@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:46:13 +0100 Subject: [PATCH 5/5] add test showcasing variants --- honeycomb-core/src/cmap/dim2/orbits.rs | 48 ++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/honeycomb-core/src/cmap/dim2/orbits.rs b/honeycomb-core/src/cmap/dim2/orbits.rs index b2bb8230e..4d7989c16 100644 --- a/honeycomb-core/src/cmap/dim2/orbits.rs +++ b/honeycomb-core/src/cmap/dim2/orbits.rs @@ -210,18 +210,34 @@ mod tests { use super::*; fn simple_map() -> CMap2 { - let mut map: CMap2 = CMap2::new(6); + let mut map: CMap2 = CMap2::new(11); + // tri1 map.force_one_link(1, 2); map.force_one_link(2, 3); map.force_one_link(3, 1); + // tri2 map.force_one_link(4, 5); map.force_one_link(5, 6); map.force_one_link(6, 4); + // pent on top + map.force_one_link(7, 8); + map.force_one_link(8, 9); + map.force_one_link(9, 10); + map.force_one_link(10, 11); + map.force_one_link(11, 7); + + // link all map.force_two_link(2, 4); + map.force_two_link(6, 7); + assert!(map.force_write_vertex(1, (0.0, 0.0)).is_none()); assert!(map.force_write_vertex(2, (1.0, 0.0)).is_none()); assert!(map.force_write_vertex(6, (1.0, 1.0)).is_none()); assert!(map.force_write_vertex(3, (0.0, 1.0)).is_none()); + assert!(map.force_write_vertex(9, (1.5, 1.5)).is_none()); + assert!(map.force_write_vertex(10, (0.5, 2.0)).is_none()); + assert!(map.force_write_vertex(11, (-0.5, 1.5)).is_none()); + map } @@ -230,9 +246,30 @@ mod tests { let map = simple_map(); let orbit = Orbit2::new(&map, OrbitPolicy::Custom(&[1, 2]), 3); let darts: Vec = orbit.collect(); - assert_eq!(darts.len(), 6); + assert_eq!(darts.len(), 11); // because the algorithm is consistent, we can predict the exact layout - assert_eq!(&darts, &[3, 1, 2, 4, 5, 6]); + assert_eq!(&darts, &[3, 1, 2, 4, 5, 6, 7, 8, 9, 10, 11]); + } + + #[test] + fn orbit_variants() { + let map = simple_map(); + + // face is complete, so everything works + let face: Vec = Orbit2::new(&map, OrbitPolicy::Face, 7).collect(); + let face_linear: Vec = Orbit2::new(&map, OrbitPolicy::FaceLinear, 7).collect(); + let face_custom: Vec = + Orbit2::new(&map, OrbitPolicy::Custom(&[0, 1]), 7).collect(); + assert_eq!(&face, &[7, 8, 11, 9, 10]); + assert_eq!(&face_linear, &[7, 8, 9, 10, 11]); + assert_eq!(&face_custom, &[7, 11, 8, 10, 9]); + + // vertex is incomplete, so using the linear variant will yield an incomplete orbit + let vertex: Vec = Orbit2::new(&map, OrbitPolicy::Vertex, 4).collect(); + let vertex_linear: Vec = + Orbit2::new(&map, OrbitPolicy::VertexLinear, 4).collect(); + assert_eq!(&vertex, &[4, 3, 7]); + assert_eq!(&vertex_linear, &[4, 3]); } #[test] @@ -266,9 +303,8 @@ mod tests { let map = simple_map(); let orbit = Orbit2::new(&map, OrbitPolicy::Vertex, 4); let darts: Vec = orbit.collect(); - // note that this one fails if we start at 3, because the vertex is not complete - assert_eq!(darts.len(), 2); - assert_eq!(&darts, &[4, 3]); + assert_eq!(darts.len(), 3); + assert_eq!(&darts, &[4, 3, 7]); } #[test]