Skip to content

Commit

Permalink
Merge branch 'master' into expand-cmap-errors
Browse files Browse the repository at this point in the history
  • Loading branch information
imrn99 committed Dec 3, 2024
2 parents 63b22a3 + 6302125 commit 7d2ae9a
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 370 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
-o target/tests.lcov
# upload results
- name: Upload reports to Codecov
uses: codecov/codecov-action@v4.6.0
uses: codecov/codecov-action@v5.0.7
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: target/tests.lcov
Expand Down
55 changes: 22 additions & 33 deletions honeycomb-core/src/attributes/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
// ------ IMPORTS

use super::{AttributeBind, AttributeStorage, AttributeUpdate, UnknownAttributeStorage};
use crate::{
cmap::{CMapError, CMapResult},
prelude::DartIdType,
};
use crate::{cmap::CMapResult, prelude::DartIdType};
use num_traits::ToPrimitive;
use stm::{atomically, StmResult, TVar, Transaction};

Expand Down Expand Up @@ -90,17 +87,17 @@ impl<A: AttributeBind + AttributeUpdate> UnknownAttributeStorage for AttrSparseV
self.data[lhs_inp as usize].read(trans)?,
self.data[rhs_inp as usize].read(trans)?,
) {
(Some(v1), Some(v2)) => Some(AttributeUpdate::merge(v1, v2)),
(Some(v), None) | (None, Some(v)) => Some(AttributeUpdate::merge_incomplete(v)),
(Some(v1), Some(v2)) => Ok(AttributeUpdate::merge(v1, v2)),
(Some(v), None) | (None, Some(v)) => AttributeUpdate::merge_incomplete(v),
(None, None) => AttributeUpdate::merge_from_none(),
};
if new_v.is_none() {
if new_v.is_err() {
eprintln!("W: cannot merge two null attribute value");
eprintln!(" setting new target value to `None`");
}
self.data[rhs_inp as usize].write(trans, None)?;
self.data[lhs_inp as usize].write(trans, None)?;
self.data[out as usize].write(trans, new_v)?;
self.data[out as usize].write(trans, new_v.ok())?;
Ok(())
}

Expand All @@ -115,25 +112,13 @@ impl<A: AttributeBind + AttributeUpdate> UnknownAttributeStorage for AttrSparseV
self.data[lhs_inp as usize].read(trans)?,
self.data[rhs_inp as usize].read(trans)?,
) {
(Some(v1), Some(v2)) => Some(AttributeUpdate::merge(v1, v2)),
(Some(_), None) | (None, Some(_)) => {
return Err(CMapError::FailedAttributeMerge(
"missing one value for merge",
))
}
(None, None) => {
return Err(CMapError::FailedAttributeMerge(
"missing both values for merge",
))
}
(Some(v1), Some(v2)) => AttributeUpdate::merge(v1, v2),
(Some(v), None) | (None, Some(v)) => AttributeUpdate::merge_incomplete(v)?,
(None, None) => AttributeUpdate::merge_from_none()?,
};
if new_v.is_none() {
eprintln!("W: cannot merge two null attribute value");
eprintln!(" setting new target value to `None`");
}
self.data[rhs_inp as usize].write(trans, None)?;
self.data[lhs_inp as usize].write(trans, None)?;
self.data[out as usize].write(trans, new_v)?;
self.data[out as usize].write(trans, Some(new_v))?;
Ok(())
}

Expand All @@ -144,8 +129,12 @@ impl<A: AttributeBind + AttributeUpdate> UnknownAttributeStorage for AttrSparseV
rhs_out: DartIdType,
inp: DartIdType,
) -> StmResult<()> {
if let Some(val) = self.data[inp as usize].read(trans)? {
let (lhs_val, rhs_val) = AttributeUpdate::split(val);
let res = if let Some(val) = self.data[inp as usize].read(trans)? {
Ok(AttributeUpdate::split(val))
} else {
AttributeUpdate::split_from_none()
};
if let Ok((lhs_val, rhs_val)) = res {
self.data[inp as usize].write(trans, None)?;
self.data[lhs_out as usize].write(trans, Some(lhs_val))?;
self.data[rhs_out as usize].write(trans, Some(rhs_val))?;
Expand All @@ -165,14 +154,14 @@ impl<A: AttributeBind + AttributeUpdate> UnknownAttributeStorage for AttrSparseV
rhs_out: DartIdType,
inp: DartIdType,
) -> CMapResult<()> {
if let Some(val) = self.data[inp as usize].read(trans)? {
let (lhs_val, rhs_val) = AttributeUpdate::split(val);
self.data[inp as usize].write(trans, None)?;
self.data[lhs_out as usize].write(trans, Some(lhs_val))?;
self.data[rhs_out as usize].write(trans, Some(rhs_val))?;
let (lhs_val, rhs_val) = if let Some(val) = self.data[inp as usize].read(trans)? {
AttributeUpdate::split(val)
} else {
return Err(CMapError::FailedAttributeSplit("no value to split from"));
}
AttributeUpdate::split_from_none()?
};
self.data[inp as usize].write(trans, None)?;
self.data[lhs_out as usize].write(trans, Some(lhs_val))?;
self.data[rhs_out as usize].write(trans, Some(rhs_val))?;
Ok(())
}
}
Expand Down
64 changes: 42 additions & 22 deletions honeycomb-core/src/attributes/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<A>()),
OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => {
$slf.vertices.get(&TypeId::of::<A>())
}
OrbitPolicy::Edge => $slf.edges.get(&TypeId::of::<A>()),
OrbitPolicy::Face => $slf.faces.get(&TypeId::of::<A>()),
OrbitPolicy::Face | OrbitPolicy::FaceLinear => $slf.faces.get(&TypeId::of::<A>()),
OrbitPolicy::Custom(_) => $slf.others.get(&TypeId::of::<A>()),
};
let $id = probably_storage
Expand All @@ -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::<A>()),
OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => {
$slf.vertices.get_mut(&TypeId::of::<A>())
}
OrbitPolicy::Edge => $slf.edges.get_mut(&TypeId::of::<A>()),
OrbitPolicy::Face => $slf.faces.get_mut(&TypeId::of::<A>()),
OrbitPolicy::Face | OrbitPolicy::FaceLinear => $slf.faces.get_mut(&TypeId::of::<A>()),
OrbitPolicy::Custom(_) => $slf.others.get_mut(&TypeId::of::<A>()),
};
let $id = probably_storage
Expand Down Expand Up @@ -162,9 +166,13 @@ impl AttrStorageManager {
let typeid = TypeId::of::<A>();
let new_storage = <A as AttributeBind>::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()
Expand Down Expand Up @@ -207,9 +215,9 @@ impl AttrStorageManager {
#[must_use = "unused getter result - please remove this method call"]
pub fn get_storage<A: AttributeBind>(&self) -> CMapResult<&<A as AttributeBind>::StorageType> {
let probably_storage = match A::BIND_POLICY {
OrbitPolicy::Vertex => &self.vertices[&TypeId::of::<A>()],
OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => &self.vertices[&TypeId::of::<A>()],
OrbitPolicy::Edge => &self.edges[&TypeId::of::<A>()],
OrbitPolicy::Face => &self.faces[&TypeId::of::<A>()],
OrbitPolicy::Face | OrbitPolicy::FaceLinear => &self.faces[&TypeId::of::<A>()],
OrbitPolicy::Custom(_) => &self.others[&TypeId::of::<A>()],
};
probably_storage
Expand All @@ -228,9 +236,11 @@ impl AttrStorageManager {
pub fn remove_storage<A: AttributeBind>(&mut self) {
// we could return it ?
let _ = match A::BIND_POLICY {
OrbitPolicy::Vertex => &self.vertices.remove(&TypeId::of::<A>()),
OrbitPolicy::Vertex | OrbitPolicy::VertexLinear => {
&self.vertices.remove(&TypeId::of::<A>())
}
OrbitPolicy::Edge => &self.edges.remove(&TypeId::of::<A>()),
OrbitPolicy::Face => &self.faces.remove(&TypeId::of::<A>()),
OrbitPolicy::Face | OrbitPolicy::FaceLinear => &self.faces.remove(&TypeId::of::<A>()),
OrbitPolicy::Custom(_) => &self.others.remove(&TypeId::of::<A>()),
};
}
Expand All @@ -253,9 +263,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!(),
}
}
Expand Down Expand Up @@ -332,11 +346,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!(),
}
}
Expand Down Expand Up @@ -445,13 +461,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!(),
Expand Down Expand Up @@ -630,11 +646,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!(),
}
}
Expand Down Expand Up @@ -714,11 +732,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!(),
}
}
Expand Down Expand Up @@ -828,13 +848,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!(),
Expand Down
14 changes: 7 additions & 7 deletions honeycomb-core/src/attributes/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use super::{
UnknownAttributeStorage,
};
use crate::{
cmap::EdgeIdType,
cmap::{CMapResult, EdgeIdType},
prelude::{CMap2, CMapBuilder, FaceIdType, OrbitPolicy, Vertex2, VertexIdType},
};
use std::any::Any;
Expand All @@ -35,12 +35,12 @@ impl AttributeUpdate for Temperature {
(attr, attr)
}

fn merge_incomplete(attr: Self) -> Self {
Temperature::from(attr.val / 2.0)
fn merge_incomplete(attr: Self) -> CMapResult<Self> {
Ok(Temperature::from(attr.val / 2.0))
}

fn merge_from_none() -> Option<Self> {
Some(Temperature::from(0.0))
fn merge_from_none() -> CMapResult<Self> {
Ok(Temperature::from(0.0))
}
}

Expand Down Expand Up @@ -449,9 +449,9 @@ fn attribute_update() {
assert_eq!(Temperature::split(t_new), (t_ref, t_ref)); // or Temperature::_
assert_eq!(
Temperature::merge_incomplete(t_ref),
Temperature::from(t_ref.val / 2.0)
Ok(Temperature::from(t_ref.val / 2.0))
);
assert_eq!(Temperature::merge_from_none(), Some(Temperature::from(0.0)));
assert_eq!(Temperature::merge_from_none(), Ok(Temperature::from(0.0)));
}

#[test]
Expand Down
Loading

0 comments on commit 7d2ae9a

Please sign in to comment.