fix: platform-stable Hash for Seq, SeqSlice, and Kmer (#15)

bio-seq/src/hash.rs

@@ -0,0 +1,37 @@
+// Copyright 2021-2024 Jeff Knaggs
+// Licensed under the MIT license (http://opensource.org/licenses/MIT)
+// This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//! Platform-stable hashing for bit-packed sequences.
+
+use crate::Bs;
+use bitvec::field::BitField;
+use core::hash::Hasher;
+
+/// Feed a bit slice into a hasher as a stable, platform-independent byte stream.
+///
+/// Bits are packed least-significant-first into bytes, matching the in-memory
+/// `Lsb0` layout. Sidestepping `bitvec`'s `Hash` impl (which routes bits
+/// through `bool::hash`, one hasher write per bit) and the `usize` storage
+/// width keeps the byte stream identical on 32- and 64-bit targets.
+#[inline]
+pub(crate) fn hash_bits<H: Hasher>(bs: &Bs, state: &mut H) {
+    // Extract whole bytes via `BitField::load_le::<u8>()`, which lets bitvec
+    // do the bit-shuffle on the underlying storage word. Buffer them so the
+    // hasher sees one bulk write per 64-byte block instead of one per byte.
+    // `load_le` zero-pads the high bits of any partial trailing chunk.
+    let mut buf = [0u8; 64];
+    let mut len = 0;
+    for chunk in bs.chunks(8) {
+        buf[len] = chunk.load_le::<u8>();
+        len += 1;
+        if len == buf.len() {
+            state.write(&buf);
+            len = 0;
+        }
+    }
+    if len > 0 {
+        state.write(&buf[..len]);
+    }
+}

bio-seq/src/kmer.rs

@@ -340,10 +340,14 @@ impl<A: Codec, const K: usize> Iterator for KmerIter<'_, A, K> {
 /// ```
 impl<A: Codec, const K: usize, S: KmerStorage> Hash for Kmer<A, K, S> {
     fn hash<H: Hasher>(&self, state: &mut H) {
+        // Length prefix as 8 little-endian bytes (see `SeqSlice` for rationale).
+        state.write(&(K as u64).to_le_bytes());
         let ba = self.bs.to_bitarray();
         let bs: &Bs = ba.as_ref();
-        bs.hash(state);
-        K.hash(state);
+        // Only the meaningful bits are hashed; the unused high bits of the
+        // storage word/array are padding and must not influence the digest.
+        let n_bits = K * A::BITS as usize;
+        crate::hash::hash_bits(&bs[..n_bits], state);
     }
 }
 

bio-seq/src/lib.rs

@@ -100,6 +100,8 @@ type Bs = BitSlice<usize, Order>;
 type Bv = BitVec<usize, Order>;
 type Ba<const W: usize> = BitArray<[usize; W], Order>;
 
+mod hash;
+
 pub mod codec;
 pub mod error;
 #[macro_use]

bio-seq/src/seq.rs

@@ -1199,6 +1199,61 @@ mod tests {
         assert_eq!(hash1, hash2);
     }
 
+    #[derive(Default)]
+    struct RecordingHasher(Vec<u8>);
+
+    impl Hasher for RecordingHasher {
+        fn finish(&self) -> u64 { 0 }
+        fn write(&mut self, bytes: &[u8]) { self.0.extend_from_slice(bytes); }
+    }
+
+    fn record<T: Hash>(value: &T) -> Vec<u8> {
+        let mut h = RecordingHasher::default();
+        value.hash(&mut h);
+        h.0
+    }
+
+    #[test]
+    fn test_hash_byte_stream_invariants() {
+        // Same logical content must produce the same byte stream regardless of
+        // how it is spelt (Seq vs &SeqSlice) and across a range of codecs and
+        // lengths — including ones with partial trailing bytes and ones long
+        // enough to cross the internal flush buffer in `hash_bits`.
+        fn check<A: Codec>(s: &str) {
+            let seq: Seq<A> = s.try_into().unwrap_or_else(|_| panic!("parse {s:?}"));
+            let bytes = record(&seq);
+            assert_eq!(record::<&SeqSlice<A>>(&&seq[..]), bytes);
+
+            // Structural: 8-byte LE length prefix, then ceil(bits / 8) bytes.
+            let n = seq.len();
+            let body = (n * A::BITS as usize).div_ceil(8);
+            assert_eq!(bytes.len(), 8 + body);
+            assert_eq!(&bytes[..8], &(n as u64).to_le_bytes());
+        }
+        check::<Dna>("A");
+        check::<Dna>("ACGT");
+        check::<Dna>("ACGTA");
+        check::<Dna>(&"ACGT".repeat(80)); // > 64-byte flush in `hash_bits`
+        check::<Iupac>("AC");
+        check::<Iupac>("ACG");
+        check::<Iupac>("NRYKBDHV");
+        check::<Amino>("MWLLP"); // 6-bit codec, partial trailing byte
+        check::<text::Dna>("ACGT"); // 8-bit codec, byte-aligned
+    }
+
+    #[test]
+    fn test_kmer_hash_independent_of_storage_width() {
+        let seq: Seq<Dna> = "ACGTACGTAC".try_into().unwrap();
+        let slice: &SeqSlice<Dna> = &seq[..];
+        let bytes = record(&slice);
+        let km_usize: Kmer<Dna, 10, usize> = slice.try_into().unwrap();
+        let km_u64: Kmer<Dna, 10, u64> = slice.try_into().unwrap();
+        let km_u128: Kmer<Dna, 10, u128> = slice.try_into().unwrap();
+        assert_eq!(record(&km_usize), bytes);
+        assert_eq!(record(&km_u64), bytes);
+        assert_eq!(record(&km_u128), bytes);
+    }
+
     #[test]
     fn test_prepend() {
         let mut seq1 =

bio-seq/src/seq/slice.rs

@@ -123,12 +123,14 @@ impl<A: Codec> PartialEq<&str> for SeqSlice<A> {
     }
 }
 
-/// Warning! hashes are not currently stable between platforms/version
 impl<A: Codec> Hash for SeqSlice<A> {
     fn hash<H: Hasher>(&self, state: &mut H) {
-        self.bs.hash(state);
-        // prepend length to make robust against matching prefixes
-        self.len().hash(state);
+        // Prefix with length as 8 little-endian bytes so the byte stream is
+        // identical on 32-/64-bit targets and across endianness. Going through
+        // `u64::hash` would route via `Hasher::write_u64`, whose default impl
+        // emits native-endian bytes.
+        state.write(&(self.len() as u64).to_le_bytes());
+        crate::hash::hash_bits(&self.bs, state);
     }
 }