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

Various refactorings #1470

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
9 changes: 4 additions & 5 deletions nalgebra-glm/src/ext/quaternion_trigonometric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ pub fn quat_angle_axis<T: RealNumber>(angle: T, axis: &TVec3<T>) -> Qua<T> {

/// The rotation axis of a quaternion assumed to be normalized.
pub fn quat_axis<T: RealNumber>(x: &Qua<T>) -> TVec3<T> {
if let Some(a) = UnitQuaternion::from_quaternion(*x).axis() {
a.into_inner()
} else {
TVec3::zeros()
}
UnitQuaternion::from_quaternion(*x)
.axis()
.map(|a| a.into_inner())
.unwrap_or_else(|| TVec3::zeros())
}
8 changes: 3 additions & 5 deletions nalgebra-glm/src/gtx/rotate_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ use crate::RealNumber;

/// Build the rotation matrix needed to align `normal` and `up`.
pub fn orientation<T: RealNumber>(normal: &TVec3<T>, up: &TVec3<T>) -> TMat4<T> {
if let Some(r) = Rotation3::rotation_between(normal, up) {
r.to_homogeneous()
} else {
TMat4::identity()
}
Rotation3::rotation_between(normal, up)
.map(|r| r.to_homogeneous())
.unwrap_or_else(|| TMat4::identity())
}

/// Rotate a two dimensional vector.
Expand Down
6 changes: 2 additions & 4 deletions nalgebra-macros/src/matrix_vector_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type MatrixRowSyntax = Punctuated<Expr, Token![,]>;

impl Parse for Matrix {
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
let mut data = Vec::new();
let mut data = vec![];
let mut ncols = None;
let mut nrows = 0;

Expand Down Expand Up @@ -126,9 +126,7 @@ impl Parse for Vector {
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
// The syntax of a vector is just the syntax of a single matrix row
if input.is_empty() {
Ok(Self {
elements: Vec::new(),
})
Ok(Self { elements: vec![] })
} else {
let elements = MatrixRowSyntax::parse_terminated(input)?
.into_iter()
Expand Down
20 changes: 10 additions & 10 deletions nalgebra-sparse/src/convert/serial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ where
S: RawStorage<T, R, C>,
{
let mut row_offsets = Vec::with_capacity(dense.nrows() + 1);
let mut col_idx = Vec::new();
let mut values = Vec::new();
let mut col_idx = vec![];
let mut values = vec![];

// We have to iterate row-by-row to build the CSR matrix, which is at odds with
// nalgebra's column-major storage. The alternative would be to perform an initial sweep
Expand Down Expand Up @@ -177,8 +177,8 @@ where
S: RawStorage<T, R, C>,
{
let mut col_offsets = Vec::with_capacity(dense.ncols() + 1);
let mut row_idx = Vec::new();
let mut values = Vec::new();
let mut row_idx = vec![];
let mut values = vec![];

col_offsets.push(0);
for j in 0..dense.ncols() {
Expand Down Expand Up @@ -268,18 +268,18 @@ where

// At this point, assembly is essentially complete. However, we must ensure
// that minor indices are sorted within each lane and without duplicates.
let mut sorted_major_offsets = Vec::new();
let mut sorted_minor_idx = Vec::new();
let mut sorted_vals = Vec::new();
let mut sorted_major_offsets = vec![];
let mut sorted_minor_idx = vec![];
let mut sorted_vals = vec![];

sorted_major_offsets.push(0);

// We need some temporary storage when working with each lane. Since lanes often have a
// very small number of non-zero entries, we try to amortize allocations across
// lanes by reusing workspace vectors
let mut idx_workspace = Vec::new();
let mut perm_workspace = Vec::new();
let mut values_workspace = Vec::new();
let mut idx_workspace = vec![];
let mut perm_workspace = vec![];
let mut values_workspace = vec![];

for lane in 0..major_dim {
let begin = unsorted_major_offsets[lane];
Expand Down
6 changes: 3 additions & 3 deletions nalgebra-sparse/src/coo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ impl<T> CooMatrix<T> {
Self {
nrows,
ncols,
row_indices: Vec::new(),
col_indices: Vec::new(),
values: Vec::new(),
row_indices: vec![],
col_indices: vec![],
values: vec![],
}
}

Expand Down
4 changes: 2 additions & 2 deletions nalgebra-sparse/src/coo/coo_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ impl<'de, T> Deserialize<'de> for CooMatrix<T>
where
T: Deserialize<'de> + Clone,
{
fn deserialize<D>(deserializer: D) -> Result<CooMatrix<T>, D::Error>
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let de = CooMatrixSerializationData::<Vec<usize>, Vec<T>>::deserialize(deserializer)?;
CooMatrix::try_from_triplets(
Self::try_from_triplets(
de.nrows,
de.ncols,
de.row_indices,
Expand Down
68 changes: 33 additions & 35 deletions nalgebra-sparse/src/cs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ impl<T> CsMatrix<T> {
{
let (major_dim, minor_dim) = (self.pattern().major_dim(), self.pattern().minor_dim());
let mut new_offsets = Vec::with_capacity(self.pattern().major_dim() + 1);
let mut new_indices = Vec::new();
let mut new_values = Vec::new();
let mut new_indices = vec![];
let mut new_values = vec![];

new_offsets.push(0);
for (i, lane) in self.lane_iter().enumerate() {
Expand Down Expand Up @@ -321,20 +321,19 @@ where
let lane = self.pattern.get_lane(self.current_lane_idx);
let minor_dim = self.pattern.minor_dim();

if let Some(minor_indices) = lane {
let count = minor_indices.len();
let values_in_lane = &self.remaining_values[..count];
self.remaining_values = &self.remaining_values[count..];
self.current_lane_idx += 1;

Some(CsLane {
minor_dim,
minor_indices,
values: values_in_lane,
})
} else {
None
}
let Some(minor_indices) = lane else {
return None;
};
let count = minor_indices.len();
let values_in_lane = &self.remaining_values[..count];
self.remaining_values = &self.remaining_values[count..];
self.current_lane_idx += 1;

Some(CsLane {
minor_dim,
minor_indices,
values: values_in_lane,
})
}
}

Expand Down Expand Up @@ -365,22 +364,21 @@ where
let lane = self.pattern.get_lane(self.current_lane_idx);
let minor_dim = self.pattern.minor_dim();

if let Some(minor_indices) = lane {
let count = minor_indices.len();

let remaining = std::mem::take(&mut self.remaining_values);
let (values_in_lane, remaining) = remaining.split_at_mut(count);
self.remaining_values = remaining;
self.current_lane_idx += 1;

Some(CsLaneMut {
minor_dim,
minor_indices,
values: values_in_lane,
})
} else {
None
}
let Some(minor_indices) = lane else {
return None;
};
let count = minor_indices.len();

let remaining = std::mem::take(&mut self.remaining_values);
let (values_in_lane, remaining) = remaining.split_at_mut(count);
self.remaining_values = remaining;
self.current_lane_idx += 1;

Some(CsLaneMut {
minor_dim,
minor_indices,
values: values_in_lane,
})
}
}

Expand Down Expand Up @@ -603,9 +601,9 @@ where
}

// Set up required buffers up front
let mut minor_idx_buffer: Vec<usize> = Vec::new();
let mut values_buffer: Vec<T> = Vec::new();
let mut minor_index_permutation: Vec<usize> = Vec::new();
let mut minor_idx_buffer: Vec<usize> = vec![];
let mut values_buffer: Vec<T> = vec![];
let mut minor_index_permutation: Vec<usize> = vec![];

// Test that each lane has strictly monotonically increasing minor indices, i.e.
// minor indices within a lane are sorted, unique. Sort minor indices within a lane if needed.
Expand Down
45 changes: 18 additions & 27 deletions nalgebra-sparse/src/csc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,30 +592,27 @@ fn pattern_format_error_to_csc_error(err: SparsityPatternFormatError) -> SparseF
use SparsityPatternFormatError::DuplicateEntry as PatternDuplicateEntry;
use SparsityPatternFormatError::*;

match err {
InvalidOffsetArrayLength => E::from_kind_and_msg(
let (kind, msg) = match err {
InvalidOffsetArrayLength => (
K::InvalidStructure,
"Length of col offset array is not equal to ncols + 1.",
),
InvalidOffsetFirstLast => E::from_kind_and_msg(
InvalidOffsetFirstLast => (
K::InvalidStructure,
"First or last col offset is inconsistent with format specification.",
),
NonmonotonicOffsets => E::from_kind_and_msg(
NonmonotonicOffsets => (
K::InvalidStructure,
"Col offsets are not monotonically increasing.",
),
NonmonotonicMinorIndices => E::from_kind_and_msg(
NonmonotonicMinorIndices => (
K::InvalidStructure,
"Row indices are not monotonically increasing (sorted) within each column.",
),
MinorIndexOutOfBounds => {
E::from_kind_and_msg(K::IndexOutOfBounds, "Row indices are out of bounds.")
}
PatternDuplicateEntry => {
E::from_kind_and_msg(K::DuplicateEntry, "Matrix data contains duplicate entries.")
}
}
MinorIndexOutOfBounds => (K::IndexOutOfBounds, "Row indices are out of bounds."),
PatternDuplicateEntry => (K::DuplicateEntry, "Matrix data contains duplicate entries."),
};
E::from_kind_and_msg(kind, msg)
}

/// Iterator type for iterating over triplets in a CSC matrix.
Expand All @@ -627,7 +624,7 @@ pub struct CscTripletIter<'a, T> {

impl<'a, T> Clone for CscTripletIter<'a, T> {
fn clone(&self) -> Self {
CscTripletIter {
Self {
pattern_iter: self.pattern_iter.clone(),
values_iter: self.values_iter.clone(),
}
Expand All @@ -649,13 +646,10 @@ impl<'a, T> Iterator for CscTripletIter<'a, T> {
type Item = (usize, usize, &'a T);

fn next(&mut self) -> Option<Self::Item> {
let next_entry = self.pattern_iter.next();
let next_value = self.values_iter.next();

match (next_entry, next_value) {
(Some((i, j)), Some(v)) => Some((j, i, v)),
_ => None,
}
self.pattern_iter
.next()
.zip(self.values_iter.next())
.map(|((i, j), v)| (j, i, v))
}
}

Expand All @@ -671,13 +665,10 @@ impl<'a, T> Iterator for CscTripletIterMut<'a, T> {

#[inline]
fn next(&mut self) -> Option<Self::Item> {
let next_entry = self.pattern_iter.next();
let next_value = self.values_mut_iter.next();

match (next_entry, next_value) {
(Some((i, j)), Some(v)) => Some((j, i, v)),
_ => None,
}
self.pattern_iter
.next()
.zip(self.values_mut_iter.next())
.map(|((i, j), v)| (j, i, v))
}
}

Expand Down
4 changes: 2 additions & 2 deletions nalgebra-sparse/src/csc/csc_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ impl<'de, T> Deserialize<'de> for CscMatrix<T>
where
T: Deserialize<'de> + Clone,
{
fn deserialize<D>(deserializer: D) -> Result<CscMatrix<T>, D::Error>
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let de = CscMatrixSerializationData::<Vec<usize>, Vec<T>>::deserialize(deserializer)?;
CscMatrix::try_from_csc_data(
Self::try_from_csc_data(
de.nrows,
de.ncols,
de.col_offsets,
Expand Down
24 changes: 9 additions & 15 deletions nalgebra-sparse/src/csr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ pub struct CsrTripletIter<'a, T> {

impl<'a, T> Clone for CsrTripletIter<'a, T> {
fn clone(&self) -> Self {
CsrTripletIter {
Self {
pattern_iter: self.pattern_iter.clone(),
values_iter: self.values_iter.clone(),
}
Expand All @@ -650,13 +650,10 @@ impl<'a, T> Iterator for CsrTripletIter<'a, T> {
type Item = (usize, usize, &'a T);

fn next(&mut self) -> Option<Self::Item> {
let next_entry = self.pattern_iter.next();
let next_value = self.values_iter.next();

match (next_entry, next_value) {
(Some((i, j)), Some(v)) => Some((i, j, v)),
_ => None,
}
self.pattern_iter
.next()
.zip(self.values_iter.next())
.map(|((i, j), v)| (i, j, v))
}
}

Expand All @@ -672,13 +669,10 @@ impl<'a, T> Iterator for CsrTripletIterMut<'a, T> {

#[inline]
fn next(&mut self) -> Option<Self::Item> {
let next_entry = self.pattern_iter.next();
let next_value = self.values_mut_iter.next();

match (next_entry, next_value) {
(Some((i, j)), Some(v)) => Some((i, j, v)),
_ => None,
}
self.pattern_iter
.next()
.zip(self.values_mut_iter.next())
.map(|((i, j), v)| (i, j, v))
}
}

Expand Down
4 changes: 2 additions & 2 deletions nalgebra-sparse/src/csr/csr_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ impl<'de, T> Deserialize<'de> for CsrMatrix<T>
where
T: Deserialize<'de> + Clone,
{
fn deserialize<D>(deserializer: D) -> Result<CsrMatrix<T>, D::Error>
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let de = CsrMatrixSerializationData::<Vec<usize>, Vec<T>>::deserialize(deserializer)?;
CsrMatrix::try_from_csr_data(
Self::try_from_csr_data(
de.nrows,
de.ncols,
de.row_offsets,
Expand Down
6 changes: 3 additions & 3 deletions nalgebra-sparse/src/factorization/cholesky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ fn reach(
marks.resize(tree.len(), false);

// TODO: avoid all those allocations.
let mut tmp = Vec::new();
let mut res = Vec::new();
let mut tmp = vec![];
let mut res = vec![];

for &irow in pattern.lane(j) {
let mut curr = irow;
Expand All @@ -329,7 +329,7 @@ fn nonzero_pattern(m: &SparsityPattern) -> (SparsityPattern, SparsityPattern) {
let (nrows, ncols) = (m.minor_dim(), m.major_dim());
let mut rows = Vec::with_capacity(m.nnz());
let mut col_offsets = Vec::with_capacity(ncols + 1);
let mut marks = Vec::new();
let mut marks = vec![];

// NOTE: the following will actually compute the non-zero pattern of
// the transpose of l.
Expand Down
Loading
Loading