MRG: safer ksize selection while still accommodating k=k*3

src/core/src/manifest.rs

@@ -82,7 +82,7 @@ impl Record {
     pub fn from_sig(sig: &Signature, path: &str) -> Vec<Self> {
         sig.iter()
             .map(|sketch| {
-                let (mut ksize, md5, with_abundance, moltype, n_hashes, num, scaled) = match sketch
+                let (mut ksize, md5, with_abundance, moltype, n_hashes, num, scaled, hash_function) = match sketch
                 {
                     Sketch::MinHash(mh) => (
                         mh.ksize() as u32,
@@ -92,6 +92,7 @@ impl Record {
                         mh.size(),
                         mh.num(),
                         mh.scaled(),
+                        mh.hash_function(),
                     ),
                     Sketch::LargeMinHash(mh) => (
                         mh.ksize() as u32,
@@ -101,15 +102,17 @@ impl Record {
                         mh.size(),
                         mh.num(),
                         mh.scaled(),
+                        mh.hash_function(),
                     ),
                     _ => unimplemented!(),
                 };
 
                 let md5short = md5[0..8].into();
 
-                if moltype != HashFunctions::Murmur64Dna {
-                    ksize /= 3;
-                }
+                ksize = match hash_function {
+                    HashFunctions::Murmur64Protein | HashFunctions::Murmur64Dayhoff | HashFunctions::Murmur64Hp => ksize / 3,
+                    _ => ksize,
+                };
 
                 Self {
                     internal_location: path.into(),
@@ -329,9 +332,12 @@ mod test {
     use tempfile::TempDir;
 
     use super::Manifest;
+    use crate::collection::Collection;
+    use crate::encodings::HashFunctions;
+    use crate::selection::{Select, Selection};
 
     #[test]
-    fn manifest_from_pathlist() {
+    fn test_manifest_from_pathlist() {
         let temp_dir = TempDir::new().unwrap();
         let utf8_output = PathBuf::from_path_buf(temp_dir.path().to_path_buf())
             .expect("Path should be valid UTF-8");
@@ -363,14 +369,14 @@ mod test {
 
     #[test]
     #[should_panic(expected = "Failed to open \"no-exist\"")]
-    fn manifest_from_pathlist_nonexistent_file() {
+    fn test_manifest_from_pathlist_nonexistent_file() {
         let filename = PathBuf::from("no-exist");
         let _manifest = Manifest::from(&filename);
     }
 
     #[test]
     #[should_panic]
-    fn manifest_from_pathlist_badfile() {
+    fn test_manifest_from_pathlist_badfile() {
         let temp_dir = TempDir::new().unwrap();
         let utf8_output = PathBuf::from_path_buf(temp_dir.path().to_path_buf())
             .expect("Path should be valid UTF-8");
@@ -388,7 +394,7 @@ mod test {
 
     #[test]
     #[should_panic]
-    fn manifest_from_paths_badpath() {
+    fn test_manifest_from_paths_badpath() {
         let base_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         let test_sigs = vec![
             PathBuf::from("no-exist"),
@@ -441,4 +447,37 @@ mod test {
             }
         }
     }
+
+    #[test]
+    fn test_manifest_selection() {
+        let base_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+
+        let test_sigs = vec![PathBuf::from("../../tests/test-data/prot/all.zip")];
+
+        let full_paths: Vec<PathBuf> = test_sigs
+            .into_iter()
+            .map(|sig| base_path.join(sig))
+            .collect();
+
+        let collection = Collection::from_zipfile(&full_paths[0]).unwrap();
+        let manifest = collection.manifest().clone();
+
+        // check selection on manifest works
+        let mut selection = Selection::default();
+        selection.set_ksize(19);
+        let prot_collect = manifest.select(&selection).unwrap();
+        // eprintln!("{}", &prot_collect);
+        assert_eq!(prot_collect.len(), 6);
+        selection.set_moltype(HashFunctions::Murmur64Protein);
+
+        let manifest = collection.manifest().clone();
+        let protein_only = manifest.select(&selection).unwrap();
+        assert_eq!(protein_only.len(), 2);
+
+        let manifest = collection.manifest().clone();
+        selection = Selection::default();
+        selection.set_scaled(100);
+        let scaled100 = manifest.select(&selection).unwrap();
+        assert_eq!(scaled100.len(), 6);
+    }
 }

src/core/src/signature.rs

@@ -801,7 +801,13 @@ impl Select for Signature {
             let mut valid = true;
             valid = if let Some(ksize) = selection.ksize() {
                 let k = s.ksize() as u32;
-                k == ksize || k == ksize * 3
+                let adjusted_ksize = match s.hash_function() {
+                    HashFunctions::Murmur64Protein
+                    | HashFunctions::Murmur64Dayhoff
+                    | HashFunctions::Murmur64Hp => ksize * 3,
+                    _ => ksize,
+                };
+                k == adjusted_ksize
             } else {
                 valid
             };
@@ -902,7 +908,7 @@ mod test {
     use crate::sketch::Sketch;
 
     #[test]
-    fn load_sig() {
+    fn test_load_sig() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/.sbt.v3/60f7e23c24a8d94791cc7a8680c493f9");
 
@@ -919,7 +925,7 @@ mod test {
     }
 
     #[test]
-    fn load_signature() {
+    fn test_load_signature() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/genome-s10+s11.sig");
 
@@ -943,7 +949,7 @@ mod test {
     }
 
     #[test]
-    fn signature_from_computeparams() {
+    fn test_signature_from_computeparams() {
         let params = ComputeParameters::builder()
             .ksizes(vec![2, 3, 4])
             .num_hashes(3u32)
@@ -960,7 +966,7 @@ mod test {
     }
 
     #[test]
-    fn signature_slow_path() {
+    fn test_signature_slow_path() {
         let params = ComputeParameters::builder()
             .ksizes(vec![2, 3, 4, 5])
             .num_hashes(3u32)
@@ -978,7 +984,7 @@ mod test {
     }
 
     #[test]
-    fn signature_add_sequence_protein() {
+    fn test_signature_add_sequence_protein() {
         let params = ComputeParameters::builder()
             .ksizes(vec![3, 6])
             .num_hashes(3u32)
@@ -996,7 +1002,7 @@ mod test {
     }
 
     #[test]
-    fn signature_add_protein() {
+    fn test_signature_add_protein() {
         let params = ComputeParameters::builder()
             .ksizes(vec![3, 6])
             .num_hashes(3u32)
@@ -1014,7 +1020,7 @@ mod test {
     }
 
     #[test]
-    fn signature_add_sequence_cp() {
+    fn test_signature_add_sequence_cp() {
         let mut cp = ComputeParameters::default();
         cp.set_dayhoff(true);
         cp.set_protein(true);
@@ -1040,7 +1046,7 @@ mod test {
     }
 
     #[test]
-    fn load_minhash_from_signature() {
+    fn test_load_minhash_from_signature() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/47.fa.sig");
 
@@ -1056,7 +1062,7 @@ mod test {
     }
 
     #[test]
-    fn load_single_sketch_from_signature() {
+    fn test_load_single_sketch_from_signature() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/47.fa.sig");
 
@@ -1080,7 +1086,7 @@ mod test {
 
     #[test]
     #[should_panic]
-    fn get_sketch_multisketch_panic() {
+    fn test_get_sketch_multisketch_panic() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/47.fa.sig");
 
@@ -1105,7 +1111,7 @@ mod test {
 
     #[test]
     #[should_panic]
-    fn load_minhash_multisketch_panic() {
+    fn test_load_minhash_multisketch_panic() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/47.fa.sig");
 
@@ -1129,7 +1135,7 @@ mod test {
     }
 
     #[test]
-    fn selection_with_downsample() {
+    fn test_selection_with_downsample() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/47+63-multisig.sig");
 
@@ -1153,7 +1159,88 @@ mod test {
     }
 
     #[test]
-    fn selection_scaled_too_low() {
+    fn test_selection_protein() {
+        let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        filename.push(
+            "../../tests/test-data/prot/protein/GCA_001593925.1_ASM159392v1_protein.faa.gz.sig",
+        );
+
+        let file = File::open(filename).unwrap();
+        let reader = BufReader::new(file);
+        let sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+
+        // create Selection object
+        let mut selection = Selection::default();
+        let prot_ksize = 19;
+        selection.set_ksize(prot_ksize);
+        let selected_sig = sigs[0].clone().select(&selection).unwrap();
+        let mh = selected_sig.minhash().unwrap();
+        assert_eq!(mh.ksize(), prot_ksize as usize * 3);
+    }
+
+    #[test]
+    fn test_selection_dayhoff() {
+        let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        filename.push(
+            "../../tests/test-data/prot/dayhoff/GCA_001593925.1_ASM159392v1_protein.faa.gz.sig",
+        );
+
+        let file = File::open(filename).unwrap();
+        let reader = BufReader::new(file);
+        let sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+
+        // create Selection object
+        let mut selection = Selection::default();
+        let prot_ksize = 19;
+        selection.set_ksize(prot_ksize);
+        selection.set_moltype(crate::encodings::HashFunctions::Murmur64Dayhoff);
+        let selected_sig = sigs[0].clone().select(&selection).unwrap();
+        let mh = selected_sig.minhash().unwrap();
+        assert_eq!(mh.ksize(), prot_ksize as usize * 3);
+    }
+
+    #[test]
+    fn test_selection_hp() {
+        let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        filename
+            .push("../../tests/test-data/prot/hp/GCA_001593925.1_ASM159392v1_protein.faa.gz.sig");
+
+        let file = File::open(filename).unwrap();
+        let reader = BufReader::new(file);
+        let sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+
+        // create Selection object
+        let mut selection = Selection::default();
+        let prot_ksize = 19;
+        selection.set_ksize(prot_ksize);
+        selection.set_moltype(crate::encodings::HashFunctions::Murmur64Hp);
+        let selected_sig = sigs[0].clone().select(&selection).unwrap();
+        let mh = selected_sig.minhash().unwrap();
+        assert_eq!(mh.ksize(), prot_ksize as usize * 3);
+    }
+
+    #[test]
+    fn test_selection_protein2() {
+        let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        filename.push(
+            "../../tests/test-data/prot/protein/GCA_001593925.1_ASM159392v1_protein.faa.gz.sig",
+        );
+
+        let file = File::open(filename).unwrap();
+        let reader = BufReader::new(file);
+        let sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+
+        // create Selection object
+        let mut selection = Selection::default();
+        let prot_ksize = 19;
+        selection.set_ksize(prot_ksize * 3);
+        let selected_sig = sigs[0].clone().select(&selection).unwrap();
+        let mh = selected_sig.minhash();
+        assert!(mh.is_none());
+    }
+
+    #[test]
+    fn test_selection_scaled_too_low() {
         let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         filename.push("../../tests/test-data/47+63-multisig.sig");