Skip to content

Commit

Permalink
fix: CodeRabbit suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
acgetchell committed Nov 9, 2024
1 parent 1233577 commit 1df83f3
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
14 changes: 10 additions & 4 deletions src/delaunay_core/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ where
/// let cell: Cell<f64, i32, &str, 3> = CellBuilder::default().vertices(vec![vertex1, vertex2, vertex3, vertex4]).data("three-one cell").build().unwrap();
/// let vertex5: Vertex<f64, i32, 3> = VertexBuilder::default().point(Point::new([0.0, 0.0, 0.0])).data(0).build().unwrap();
/// let cell2: Cell<f64, i32, &str, 3> = CellBuilder::default().vertices(vec![vertex1, vertex2, vertex3, vertex5]).data("one-three cell").build().unwrap();
/// assert!(cell.contains_vertex_of(cell2));
pub fn contains_vertex_of(&self, cell: Cell<T, U, V, D>) -> bool {
/// assert!(cell.contains_vertex_of(&cell2));
pub fn contains_vertex_of(&self, cell: &Cell<T, U, V, D>) -> bool {
self.vertices.iter().any(|v| cell.vertices.contains(v))
}

Expand Down Expand Up @@ -406,7 +406,9 @@ where

/// The function `circumsphere_contains_vertex` checks if a given vertex is
/// contained in the circumsphere of the Cell using a matrix determinant.
/// It should be more numerically stable than `circumsphere_contains`.
/// This method is preferred over `circumsphere_contains` as it provides better numerical
/// stability by using a matrix determinant approach instead of distance calculations,
/// which can accumulate floating-point errors.
///
/// # Arguments:
///
Expand Down Expand Up @@ -499,6 +501,10 @@ where
/// Returns `true` if the given facet is contained in the cell, and `false` otherwise.
pub fn contains_facet(&self, facet: &Facet<T, U, V, D>) -> bool {
self.vertices.iter().all(|v| facet.vertices().contains(v))

// Alternative implementation using HashSet, requires Eq on T in Vertex<T, U, D>
// let vertex_set: HashSet<&Vertex<T, U, D>> = self.vertices.iter().collect();
// facet.vertices().iter().all(|v| vertex_set.contains(v))
}
}

Expand Down Expand Up @@ -901,7 +907,7 @@ mod tests {
.build()
.unwrap();

assert!(cell.contains_vertex_of(cell2));
assert!(cell.contains_vertex_of(&cell2));

// Human readable output for cargo test -- --nocapture
println!("Cell: {:?}", cell);
Expand Down
10 changes: 6 additions & 4 deletions src/delaunay_core/facet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,12 @@ where
/// The `vertices` method in the [Facet] returns a container of
/// [Vertex] objects that are in the [Facet].
pub fn vertices(&self) -> Vec<Vertex<T, U, D>> {
let mut vertices = self.cell.clone().vertices;
vertices.retain(|v| *v != self.vertex);

vertices
self.cell
.vertices
.iter()
.filter(|v| **v != self.vertex)
.cloned()
.collect()
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/delaunay_core/triangulation_data_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,14 @@ where
self.cells.insert(supercell.uuid, supercell.clone());

// Iterate over vertices
for vertex in self.vertices.values().cloned().collect::<Vec<_>>() {
for vertex in self.vertices.values() {
let mut bad_cells = Vec::new();
let mut boundary_facets = Vec::new();

// Find cells whose circumsphere contains the vertex
for (cell_id, cell) in self.cells.iter() {
// if cell.circumsphere_contains(vertex)? {
if cell.circumsphere_contains_vertex(vertex)? {
if cell.circumsphere_contains_vertex(*vertex)? {
bad_cells.push(*cell_id);
}
}
Expand All @@ -297,14 +297,14 @@ where

// Create new cells using the boundary facets and the new vertex
for facet in boundary_facets {
let new_cell = Cell::from_facet_and_vertex(facet, vertex)?;
let new_cell = Cell::from_facet_and_vertex(facet, *vertex)?;
self.cells.insert(new_cell.uuid, new_cell);
}
}

// Remove cells that contain vertices of the supercell
self.cells
.retain(|_, cell| !cell.contains_vertex_of(supercell.clone()));
.retain(|_, cell| !cell.contains_vertex_of(&supercell));

Ok(self.clone())
}
Expand Down Expand Up @@ -466,7 +466,7 @@ mod tests {
});

assert_eq!(result.number_of_vertices(), 4);
assert_eq!(result.number_of_cells(), 0);
assert_eq!(result.number_of_cells(), 1);

// Human readable output for cargo test -- --nocapture
println!("{:?}", result);
Expand Down

0 comments on commit 1df83f3

Please sign in to comment.