Skip to content

Commit

Permalink
bug fixes and cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
luizirber committed Aug 29, 2022
1 parent 370c4ff commit 42dbd08
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 54 deletions.
35 changes: 8 additions & 27 deletions src/core/src/ffi/minhash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,15 +540,13 @@ impl MinHash {
MinHash::Frozen(ref ot) => {
Into::<KmerMinHash>::into(mh.clone()).count_common(ot, downsample)
}
_ => Err(Error::MismatchSignatureType),
},

MinHash::Frozen(ref mh) => match other {
MinHash::Frozen(ref ot) => mh.count_common(ot, downsample),
MinHash::Mutable(ref ot) => {
Into::<KmerMinHashBTree>::into(mh.clone()).count_common(ot, downsample)
}
_ => Err(Error::MismatchSignatureType),
},
}
}
Expand All @@ -557,14 +555,12 @@ impl MinHash {
match *self {
MinHash::Mutable(ref mut mh) => match other {
MinHash::Mutable(ref ot) => mh.merge(ot),
MinHash::Frozen(ref ot) => Into::<KmerMinHash>::into(mh.clone()).merge(ot),
_ => Err(Error::MismatchSignatureType),
MinHash::Frozen(ref ot) => mh.merge(&Into::<KmerMinHashBTree>::into(ot.clone())),
},

MinHash::Frozen(ref mut mh) => match other {
MinHash::Frozen(ref ot) => mh.merge(ot),
MinHash::Mutable(ref ot) => Into::<KmerMinHashBTree>::into(mh.clone()).merge(ot),
_ => Err(Error::MismatchSignatureType),
MinHash::Mutable(ref ot) => mh.merge(&Into::<KmerMinHash>::into(ot.clone())),
},
}
}
Expand All @@ -583,15 +579,13 @@ impl MinHash {
ignore_abundance,
downsample,
),
_ => Err(Error::MismatchSignatureType),
},

MinHash::Frozen(ref mh) => {
match other {
MinHash::Frozen(ref ot) => mh.similarity(ot, ignore_abundance, downsample),
MinHash::Mutable(ref ot) => Into::<KmerMinHashBTree>::into(mh.clone())
.similarity(ot, ignore_abundance, downsample),
_ => Err(Error::MismatchSignatureType),
}
}
}
Expand All @@ -602,13 +596,11 @@ impl MinHash {
MinHash::Mutable(ref mh) => match other {
MinHash::Mutable(ref ot) => mh.jaccard(ot),
MinHash::Frozen(ref ot) => Into::<KmerMinHash>::into(mh.clone()).jaccard(ot),
_ => Err(Error::MismatchSignatureType),
},

MinHash::Frozen(ref mh) => match other {
MinHash::Frozen(ref ot) => mh.jaccard(ot),
MinHash::Mutable(ref ot) => Into::<KmerMinHashBTree>::into(mh.clone()).jaccard(ot),
_ => Err(Error::MismatchSignatureType),
},
}
}
Expand All @@ -620,15 +612,13 @@ impl MinHash {
MinHash::Frozen(ref ot) => {
Into::<KmerMinHash>::into(mh.clone()).intersection_size(ot)
}
_ => Err(Error::MismatchSignatureType),
},

MinHash::Frozen(ref mh) => match other {
MinHash::Frozen(ref ot) => mh.intersection_size(ot),
MinHash::Mutable(ref ot) => {
Into::<KmerMinHashBTree>::into(mh.clone()).intersection_size(ot)
}
_ => Err(Error::MismatchSignatureType),
},
}
}
Expand All @@ -638,15 +628,13 @@ impl MinHash {
MinHash::Mutable(ref mh) => match other {
MinHash::Mutable(ref ot) => mh.intersection(ot),
MinHash::Frozen(ref ot) => Into::<KmerMinHash>::into(mh.clone()).intersection(ot),
_ => Err(Error::MismatchSignatureType),
},

MinHash::Frozen(ref mh) => match other {
MinHash::Frozen(ref ot) => mh.intersection(ot),
MinHash::Mutable(ref ot) => {
Into::<KmerMinHashBTree>::into(mh.clone()).intersection(ot)
}
_ => Err(Error::MismatchSignatureType),
},
}
}
Expand All @@ -655,14 +643,12 @@ impl MinHash {
match *self {
MinHash::Mutable(ref mut mh) => match other {
MinHash::Mutable(ref ot) => mh.add_from(ot),
MinHash::Frozen(ref ot) => Into::<KmerMinHash>::into(mh.clone()).add_from(ot),
_ => Err(Error::MismatchSignatureType),
MinHash::Frozen(ref ot) => mh.add_from(&Into::<KmerMinHashBTree>::into(ot.clone())),
},

MinHash::Frozen(ref mut mh) => match other {
MinHash::Frozen(ref ot) => mh.add_from(ot),
MinHash::Mutable(ref ot) => Into::<KmerMinHashBTree>::into(mh.clone()).add_from(ot),
_ => Err(Error::MismatchSignatureType),
MinHash::Mutable(ref ot) => mh.add_from(&Into::<KmerMinHash>::into(ot.clone())),
},
}
}
Expand All @@ -671,16 +657,14 @@ impl MinHash {
match *self {
MinHash::Mutable(ref mut mh) => match other {
MinHash::Mutable(ref ot) => mh.remove_from(ot),
MinHash::Frozen(ref ot) => Into::<KmerMinHash>::into(mh.clone()).remove_from(ot),
_ => Err(Error::MismatchSignatureType),
MinHash::Frozen(ref ot) => {
mh.remove_from(&Into::<KmerMinHashBTree>::into(ot.clone()))
}
},

MinHash::Frozen(ref mut mh) => match other {
MinHash::Frozen(ref ot) => mh.remove_from(ot),
MinHash::Mutable(ref ot) => {
Into::<KmerMinHashBTree>::into(mh.clone()).remove_from(ot)
}
_ => Err(Error::MismatchSignatureType),
MinHash::Mutable(ref ot) => mh.remove_from(&Into::<KmerMinHash>::into(ot.clone())),
},
}
}
Expand All @@ -689,7 +673,6 @@ impl MinHash {
impl From<MinHash> for Sketch {
fn from(mh: MinHash) -> Sketch {
match mh {
// TODO: avoid clone
MinHash::Mutable(mh) => Sketch::LargeMinHash(mh),
MinHash::Frozen(mh) => Sketch::MinHash(mh),
}
Expand Down Expand Up @@ -871,15 +854,13 @@ impl SigsTrait for MinHash {
MinHash::Frozen(ref ot) => {
Into::<KmerMinHash>::into(mh.clone()).check_compatible(ot)
}
_ => Err(Error::MismatchSignatureType),
},

MinHash::Frozen(ref mh) => match other {
MinHash::Frozen(ref ot) => mh.check_compatible(ot),
MinHash::Mutable(ref ot) => {
Into::<KmerMinHashBTree>::into(mh.clone()).check_compatible(ot)
}
_ => Err(Error::MismatchSignatureType),
},
}
}
Expand Down
1 change: 0 additions & 1 deletion src/core/src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ pub mod index;
pub mod minhash;
pub mod nodegraph;
pub mod signature;
pub mod sketch;
pub mod storage;

use std::ffi::CStr;
Expand Down
1 change: 0 additions & 1 deletion src/core/src/ffi/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::sketch::Sketch;

use crate::ffi::cmd::compute::SourmashComputeParameters;
use crate::ffi::minhash::{MinHash, SourmashKmerMinHash};
use crate::ffi::sketch::SourmashSketch;
use crate::ffi::utils::{ForeignObject, SourmashStr};

pub struct SourmashSignature;
Expand Down
13 changes: 0 additions & 13 deletions src/core/src/ffi/sketch.rs

This file was deleted.

27 changes: 20 additions & 7 deletions src/core/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,8 @@ impl Default for Signature {

impl PartialEq for Signature {
fn eq(&self, other: &Signature) -> bool {
use crate::sketch::minhash::{KmerMinHash, KmerMinHashBTree};

let metadata = self.class == other.class
&& self.email == other.email
&& self.hash_function == other.hash_function
Expand All @@ -803,14 +805,25 @@ impl PartialEq for Signature {

// TODO: find the right signature
// as long as we have a matching
if let Sketch::MinHash(mh) = &self.signatures[0] {
if let Sketch::MinHash(other_mh) = &other.signatures[0] {
return metadata && (mh == other_mh);
}
} else {
unimplemented!()
match &self.signatures[0] {
Sketch::MinHash(mh) => match &other.signatures[0] {
Sketch::MinHash(other_mh) => return metadata && (mh == other_mh),
Sketch::LargeMinHash(other_mh) => {
// TODO: avoid clone
metadata && (mh == &Into::<KmerMinHash>::into(other_mh.clone()))
}
Sketch::HyperLogLog(_) => todo!(),
},
Sketch::LargeMinHash(mh) => match &other.signatures[0] {
Sketch::LargeMinHash(other_mh) => return metadata && (mh == other_mh),
Sketch::MinHash(other_mh) => {
// TODO: avoid clone
metadata && (mh == &Into::<KmerMinHashBTree>::into(other_mh.clone()))
}
Sketch::HyperLogLog(_) => todo!(),
},
Sketch::HyperLogLog(_) => todo!(),
}
metadata
}
}

Expand Down
22 changes: 17 additions & 5 deletions src/core/src/sketch/minhash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ pub trait AbundMinHashOps: MinHashOps {
match k.cmp(&hash) {
Ordering::Less => next_hash = other_iter.next(),
Ordering::Equal => {
unsafe {
prod += abund * other_abund;
}
prod += abund * other_abund;
break;
}
Ordering::Greater => break,
Expand Down Expand Up @@ -786,6 +784,11 @@ impl AbundMinHashOps for KmerMinHash {
}

fn set_hash_with_abundance(&mut self, hash: u64, abundance: u64) {
if abundance == 0 {
self.remove_hash(hash);
return;
}

let mut found = false;
if let Ok(pos) = self.mins.binary_search(&hash) {
if self.mins[pos] == hash {
Expand Down Expand Up @@ -842,6 +845,7 @@ impl SigsTrait for KmerMinHash {
fn hash_function(&self) -> HashFunctions {
self.hash_function
}

fn set_hash_function(&mut self, h: HashFunctions) -> Result<(), Error> {
if self.hash_function == h {
return Ok(());
Expand Down Expand Up @@ -1465,7 +1469,7 @@ impl AbundMinHashOps for KmerMinHashBTree {
}

if abundance == 0 {
// well, don't add it.
self.remove_hash(hash);
return;
}

Expand Down Expand Up @@ -1510,9 +1514,17 @@ impl AbundMinHashOps for KmerMinHashBTree {
}

fn set_hash_with_abundance(&mut self, hash: u64, abundance: u64) {
if abundance == 0 {
self.remove_hash(hash);
return;
}

if self.mins.contains(&hash) {
if let Some(ref mut abunds) = self.abunds {
abunds.get_mut(&hash).map(|v| *v = abundance);
abunds
.entry(hash)
.and_modify(|v| *v = abundance)
.or_insert_with(|| abundance);
}
} else {
self.add_hash_with_abundance(hash, abundance);
Expand Down

0 comments on commit 42dbd08

Please sign in to comment.