From 04f013d6afdfffb2e3e034cbbd94279078966ccb Mon Sep 17 00:00:00 2001 From: Martin Larralde <martin.larralde@embl.de> Date: Mon, 2 Sep 2024 20:40:48 +0200 Subject: [PATCH] Fix some Rust code issues in `lightmotif` --- lightmotif-io/src/transfac/parse.rs | 8 +-- lightmotif/src/dense.rs | 5 +- lightmotif/src/pli/mod.rs | 9 ++-- lightmotif/src/pli/platform/avx2.rs | 77 +++++++++++++++-------------- lightmotif/src/pli/platform/neon.rs | 4 +- lightmotif/src/pli/platform/sse2.rs | 6 +-- lightmotif/src/pwm.rs | 10 +++- lightmotif/src/scan.rs | 4 +- lightmotif/src/seq.rs | 32 ++++++------ 9 files changed, 80 insertions(+), 75 deletions(-) diff --git a/lightmotif-io/src/transfac/parse.rs b/lightmotif-io/src/transfac/parse.rs index 00e43bf..801a26e 100644 --- a/lightmotif-io/src/transfac/parse.rs +++ b/lightmotif-io/src/transfac/parse.rs @@ -186,10 +186,10 @@ pub fn parse_reference(mut input: &str) -> IResult<&str, Reference> { pub fn parse_record<A: Alphabet>(mut input: &str) -> IResult<&str, Record<A>> { let mut accession = None; - let mut ba = None; + let mut _ba = None; let mut name = None; let mut id = None; - let mut copyright = None; + let mut _copyright = None; let mut description = None; let mut dates = Vec::new(); let mut references = Vec::new(); @@ -207,7 +207,7 @@ pub fn parse_record<A: Alphabet>(mut input: &str) -> IResult<&str, Record<A>> { } "BA" => { let (rest, line) = preceded(tag("BA"), parse_line)(input)?; - ba = Some(line.trim().to_string()); // FIXME: check uniqueness? + _ba = Some(line.trim().to_string()); // FIXME: check uniqueness? input = rest; } "BS" => { @@ -227,7 +227,7 @@ pub fn parse_record<A: Alphabet>(mut input: &str) -> IResult<&str, Record<A>> { } "CO" => { let (rest, line) = preceded(tag("CO"), parse_line)(input)?; - copyright = Some(line.trim().to_string()); // FIXME: check uniqueness? + _copyright = Some(line.trim().to_string()); // FIXME: check uniqueness? input = rest; } "DE" => { diff --git a/lightmotif/src/dense.rs b/lightmotif/src/dense.rs index 376a20d..66b8977 100644 --- a/lightmotif/src/dense.rs +++ b/lightmotif/src/dense.rs @@ -8,8 +8,6 @@ use std::ops::Index; use std::ops::IndexMut; use std::ops::Range; -use crate::abc::Alphabet; -use crate::abc::Symbol; use crate::num::ArrayLength; // --- MatrixElement ----------------------------------------------------------- @@ -60,6 +58,7 @@ impl<T: MatrixElement, C: ArrayLength> DenseMatrix<T, C> { } /// Create a new *uninitialized* matrix with the given number of rows. + #[allow(clippy::uninit_vec)] pub unsafe fn uninitialized(rows: usize) -> Self { let mut m = Self::new(0); m.data.reserve(rows); @@ -219,7 +218,7 @@ impl<'a, T: MatrixElement, C: ArrayLength> IntoIterator for &'a mut DenseMatrix< } } -impl<'a, T: MatrixElement + PartialEq, C: ArrayLength> PartialEq for DenseMatrix<T, C> { +impl<T: MatrixElement + PartialEq, C: ArrayLength> PartialEq for DenseMatrix<T, C> { fn eq(&self, other: &Self) -> bool { if self.rows() != other.rows() { return false; diff --git a/lightmotif/src/pli/mod.rs b/lightmotif/src/pli/mod.rs index f10c829..dcc8e2c 100644 --- a/lightmotif/src/pli/mod.rs +++ b/lightmotif/src/pli/mod.rs @@ -36,6 +36,7 @@ pub mod platform; /// Used for encoding a sequence into rank-based encoding. pub trait Encode<A: Alphabet> { /// Encode the given sequence into a vector of symbols. + #[allow(clippy::uninit_vec)] fn encode_raw<S: AsRef<[u8]>>(&self, seq: S) -> Result<Vec<A::Symbol>, InvalidSymbol> { let s = seq.as_ref(); let mut buffer = Vec::with_capacity(s.len()); @@ -84,7 +85,7 @@ pub trait Score<T: MatrixElement + AddAssign, A: Alphabet, C: StrictlyPositive + let seq = seq.as_ref(); let pssm = pssm.as_ref(); - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } @@ -115,7 +116,7 @@ pub trait Score<T: MatrixElement + AddAssign, A: Alphabet, C: StrictlyPositive + { let s = seq.as_ref(); let rows = s.matrix().rows() - s.wrap(); - Self::score_rows_into(&self, pssm, s, 0..rows, scores) + self.score_rows_into(pssm, s, 0..rows, scores) } /// Compute the PSSM scores for every sequence positions. @@ -272,14 +273,14 @@ impl<A: Alphabet> Pipeline<A, Dispatch> { alphabet: std::marker::PhantomData, }; } - #[cfg(any(target_arch = "x86"))] + #[cfg(target_arch = "x86")] if std::is_x86_feature_detected!("sse2") { return Self { backend: Dispatch::Sse2, alphabet: std::marker::PhantomData, }; } - #[cfg(any(target_arch = "x86_64"))] + #[cfg(target_arch = "x86_64")] return Self { backend: Dispatch::Sse2, alphabet: std::marker::PhantomData, diff --git a/lightmotif/src/pli/platform/avx2.rs b/lightmotif/src/pli/platform/avx2.rs index 36c31f8..a0990ca 100644 --- a/lightmotif/src/pli/platform/avx2.rs +++ b/lightmotif/src/pli/platform/avx2.rs @@ -84,8 +84,8 @@ where // If an invalid symbol was encountered, recover which one. // FIXME: run a vectorize the error search? if _mm256_testz_si256(error, error) != 1 { - for i in 0..l { - A::Symbol::from_ascii(seq[i])?; + for s in seq.iter() { + A::Symbol::from_ascii(*s)?; } } @@ -540,6 +540,7 @@ unsafe fn max_u8_avx2(scores: &StripedScores<u8, <Avx2 as Backend>::LANES>) -> O #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] #[target_feature(enable = "avx2")] +#[allow(clippy::erasing_op, clippy::identity_op)] unsafe fn stripe_avx2<A>( seq: &[A::Symbol], striped: &mut StripedSequence<A, <Avx2 as Backend>::LANES>, @@ -598,38 +599,38 @@ unsafe fn stripe_avx2<A>( // Process sequence block by block let mut i = 0; while i + <Avx2 as Backend>::LANES::USIZE <= src_stride { - let mut r00 = _mm256_loadu_si256(src.add(00 * src_stride) as _); - let mut r01 = _mm256_loadu_si256(src.add(src_stride) as _); - let mut r02 = _mm256_loadu_si256(src.add(02 * src_stride) as _); - let mut r03 = _mm256_loadu_si256(src.add(03 * src_stride) as _); - let mut r04 = _mm256_loadu_si256(src.add(04 * src_stride) as _); - let mut r05 = _mm256_loadu_si256(src.add(05 * src_stride) as _); - let mut r06 = _mm256_loadu_si256(src.add(06 * src_stride) as _); - let mut r07 = _mm256_loadu_si256(src.add(07 * src_stride) as _); - let mut r08 = _mm256_loadu_si256(src.add(08 * src_stride) as _); - let mut r09 = _mm256_loadu_si256(src.add(09 * src_stride) as _); - let mut r10 = _mm256_loadu_si256(src.add(10 * src_stride) as _); - let mut r11 = _mm256_loadu_si256(src.add(11 * src_stride) as _); - let mut r12 = _mm256_loadu_si256(src.add(12 * src_stride) as _); - let mut r13 = _mm256_loadu_si256(src.add(13 * src_stride) as _); - let mut r14 = _mm256_loadu_si256(src.add(14 * src_stride) as _); - let mut r15 = _mm256_loadu_si256(src.add(15 * src_stride) as _); - let mut r16 = _mm256_loadu_si256(src.add(16 * src_stride) as _); - let mut r17 = _mm256_loadu_si256(src.add(17 * src_stride) as _); - let mut r18 = _mm256_loadu_si256(src.add(18 * src_stride) as _); - let mut r19 = _mm256_loadu_si256(src.add(19 * src_stride) as _); - let mut r20 = _mm256_loadu_si256(src.add(20 * src_stride) as _); - let mut r21 = _mm256_loadu_si256(src.add(21 * src_stride) as _); - let mut r22 = _mm256_loadu_si256(src.add(22 * src_stride) as _); - let mut r23 = _mm256_loadu_si256(src.add(23 * src_stride) as _); - let mut r24 = _mm256_loadu_si256(src.add(24 * src_stride) as _); - let mut r25 = _mm256_loadu_si256(src.add(25 * src_stride) as _); - let mut r26 = _mm256_loadu_si256(src.add(26 * src_stride) as _); - let mut r27 = _mm256_loadu_si256(src.add(27 * src_stride) as _); - let mut r28 = _mm256_loadu_si256(src.add(28 * src_stride) as _); - let mut r29 = _mm256_loadu_si256(src.add(29 * src_stride) as _); - let mut r30 = _mm256_loadu_si256(src.add(30 * src_stride) as _); - let mut r31 = _mm256_loadu_si256(src.add(31 * src_stride) as _); + let mut r00 = _mm256_loadu_si256(src.add(0x00 * src_stride) as _); + let mut r01 = _mm256_loadu_si256(src.add(0x01 * src_stride) as _); + let mut r02 = _mm256_loadu_si256(src.add(0x02 * src_stride) as _); + let mut r03 = _mm256_loadu_si256(src.add(0x03 * src_stride) as _); + let mut r04 = _mm256_loadu_si256(src.add(0x04 * src_stride) as _); + let mut r05 = _mm256_loadu_si256(src.add(0x05 * src_stride) as _); + let mut r06 = _mm256_loadu_si256(src.add(0x06 * src_stride) as _); + let mut r07 = _mm256_loadu_si256(src.add(0x07 * src_stride) as _); + let mut r08 = _mm256_loadu_si256(src.add(0x08 * src_stride) as _); + let mut r09 = _mm256_loadu_si256(src.add(0x09 * src_stride) as _); + let mut r10 = _mm256_loadu_si256(src.add(0x0a * src_stride) as _); + let mut r11 = _mm256_loadu_si256(src.add(0x0b * src_stride) as _); + let mut r12 = _mm256_loadu_si256(src.add(0x0c * src_stride) as _); + let mut r13 = _mm256_loadu_si256(src.add(0x0d * src_stride) as _); + let mut r14 = _mm256_loadu_si256(src.add(0x0e * src_stride) as _); + let mut r15 = _mm256_loadu_si256(src.add(0x0f * src_stride) as _); + let mut r16 = _mm256_loadu_si256(src.add(0x10 * src_stride) as _); + let mut r17 = _mm256_loadu_si256(src.add(0x11 * src_stride) as _); + let mut r18 = _mm256_loadu_si256(src.add(0x12 * src_stride) as _); + let mut r19 = _mm256_loadu_si256(src.add(0x13 * src_stride) as _); + let mut r20 = _mm256_loadu_si256(src.add(0x14 * src_stride) as _); + let mut r21 = _mm256_loadu_si256(src.add(0x15 * src_stride) as _); + let mut r22 = _mm256_loadu_si256(src.add(0x16 * src_stride) as _); + let mut r23 = _mm256_loadu_si256(src.add(0x17 * src_stride) as _); + let mut r24 = _mm256_loadu_si256(src.add(0x18 * src_stride) as _); + let mut r25 = _mm256_loadu_si256(src.add(0x19 * src_stride) as _); + let mut r26 = _mm256_loadu_si256(src.add(0x1a * src_stride) as _); + let mut r27 = _mm256_loadu_si256(src.add(0x1b * src_stride) as _); + let mut r28 = _mm256_loadu_si256(src.add(0x1c * src_stride) as _); + let mut r29 = _mm256_loadu_si256(src.add(0x1d * src_stride) as _); + let mut r30 = _mm256_loadu_si256(src.add(0x1e * src_stride) as _); + let mut r31 = _mm256_loadu_si256(src.add(0x1f * src_stride) as _); unpack!(epi8, r00, r01); unpack!(epi8, r02, r03); @@ -717,7 +718,7 @@ unsafe fn stripe_avx2<A>( unpack!(epi128, r15, r31); _mm256_stream_si256(out.add(0x00 * out_stride) as _, r00); - _mm256_stream_si256(out.add(out_stride) as _, r08); + _mm256_stream_si256(out.add(0x01 * out_stride) as _, r08); _mm256_stream_si256(out.add(0x02 * out_stride) as _, r04); _mm256_stream_si256(out.add(0x03 * out_stride) as _, r12); _mm256_stream_si256(out.add(0x04 * out_stride) as _, r02); @@ -816,7 +817,7 @@ impl Avx2 { ); } - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } @@ -851,7 +852,7 @@ impl Avx2 { ); } - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } @@ -888,7 +889,7 @@ impl Avx2 { ); } - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } diff --git a/lightmotif/src/pli/platform/neon.rs b/lightmotif/src/pli/platform/neon.rs index 2bddb8a..0fb7f9c 100644 --- a/lightmotif/src/pli/platform/neon.rs +++ b/lightmotif/src/pli/platform/neon.rs @@ -264,7 +264,7 @@ impl Neon { ); } - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } @@ -302,7 +302,7 @@ impl Neon { ); } - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } diff --git a/lightmotif/src/pli/platform/sse2.rs b/lightmotif/src/pli/platform/sse2.rs index fad275f..274201a 100644 --- a/lightmotif/src/pli/platform/sse2.rs +++ b/lightmotif/src/pli/platform/sse2.rs @@ -87,8 +87,8 @@ where let mut x: [u8; 16] = [0; 16]; _mm_storeu_si128(x.as_mut_ptr() as *mut __m128i, error); if x.iter().any(|&x| x != 0) { - for i in 0..l { - let _ = A::Symbol::from_ascii(seq[i])?; + for x in seq.iter() { + let _ = A::Symbol::from_ascii(*x)?; } } @@ -292,7 +292,7 @@ impl Sse2 { ); } - if seq.len() < pssm.rows() || rows.len() == 0 { + if seq.len() < pssm.rows() || rows.is_empty() { scores.resize(0, 0); return; } diff --git a/lightmotif/src/pwm.rs b/lightmotif/src/pwm.rs index 432225b..adfcf28 100644 --- a/lightmotif/src/pwm.rs +++ b/lightmotif/src/pwm.rs @@ -28,11 +28,17 @@ macro_rules! matrix_traits { &self.data } - /// The length of the motif encoded in this matrix. + /// Get the length of the motif encoded in this matrix. #[inline] pub const fn len(&self) -> usize { self.data.rows() } + + /// Check whether the matrix is empty. + #[inline] + pub const fn is_empty(&self) -> bool { + self.data.rows() == 0 + } } impl<A: Alphabet> AsRef<$mx<A>> for $mx<A> { @@ -115,7 +121,7 @@ impl<A: Alphabet> CountMatrix<A> { /// ]); /// assert!(result.is_err()); /// ``` - pub fn from_sequences<'seq, I>(sequences: I) -> Result<Self, InvalidData> + pub fn from_sequences<I>(sequences: I) -> Result<Self, InvalidData> where I: IntoIterator, <I as IntoIterator>::Item: AsRef<EncodedSequence<A>>, diff --git a/lightmotif/src/scan.rs b/lightmotif/src/scan.rs index 4123526..c67e524 100644 --- a/lightmotif/src/scan.rs +++ b/lightmotif/src/scan.rs @@ -40,7 +40,7 @@ impl<T> std::ops::Deref for CowMut<'_, T> { fn deref(&self) -> &T { match self { CowMut::Owned(it) => it, - CowMut::Borrowed(it) => *it, + CowMut::Borrowed(it) => it, } } } @@ -49,7 +49,7 @@ impl<T> std::ops::DerefMut for CowMut<'_, T> { fn deref_mut(&mut self) -> &mut T { match self { CowMut::Owned(it) => it, - CowMut::Borrowed(it) => *it, + CowMut::Borrowed(it) => it, } } } diff --git a/lightmotif/src/seq.rs b/lightmotif/src/seq.rs index b91bb6f..7e7043d 100644 --- a/lightmotif/src/seq.rs +++ b/lightmotif/src/seq.rs @@ -3,6 +3,7 @@ use std::fmt::Display; use std::fmt::Formatter; use std::fmt::Result as FmtResult; +use std::fmt::Write; use std::ops::Index; use std::str::FromStr; @@ -80,6 +81,12 @@ impl<A: Alphabet> EncodedSequence<A> { self.data.len() } + /// Check whether the sequence is empty. + #[inline] + pub fn is_empty(&self) -> bool { + self.data.is_empty() + } + /// Iterate over the symbols in the sequence. #[inline] pub fn iter(&self) -> <&[A::Symbol] as IntoIterator>::IntoIter { @@ -98,15 +105,6 @@ impl<A: Alphabet> EncodedSequence<A> { let pli = Pipeline::<A, _>::dispatch(); pli.stripe(&self.data) } - - /// Convert the encoded sequence back to its textual representation. - pub fn to_string(&self) -> String { - let mut s = String::with_capacity(self.len()); - for c in self.data.iter() { - s.push(c.as_char()); - } - s - } } impl<A: Alphabet> AsRef<EncodedSequence<A>> for EncodedSequence<A> { @@ -130,7 +128,7 @@ impl<A: Alphabet> Default for EncodedSequence<A> { impl<A: Alphabet> Display for EncodedSequence<A> { fn fmt(&self, f: &mut Formatter) -> FmtResult { for c in self.data.iter() { - write!(f, "{}", c.as_char())?; + f.write_char(c.as_char())?; } Ok(()) } @@ -184,12 +182,6 @@ where let r = other.as_ref(); l == r } - - fn ne(&self, other: &S) -> bool { - let l = self.data.as_slice(); - let r = other.as_ref(); - l != r - } } // --- StripedSequence --------------------------------------------------------- @@ -243,6 +235,12 @@ impl<A: Alphabet, C: StrictlyPositive + ArrayLength> StripedSequence<A, C> { self.length } + /// Check whether the sequence is empty. + #[inline] + pub const fn is_empty(&self) -> bool { + self.length == 0 + } + /// Get the number of wrapping rows in the striped matrix. #[inline] pub const fn wrap(&self) -> usize { @@ -263,7 +261,7 @@ impl<A: Alphabet, C: StrictlyPositive + ArrayLength> StripedSequence<A, C> { /// Reconfigure the striped sequence for searching with a motif. pub fn configure(&mut self, motif: &ScoringMatrix<A>) { - if motif.len() > 0 { + if !motif.is_empty() { self.configure_wrap(motif.len() - 1); } } -- GitLab