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

AVX2 and AVX512 implementations for ntHash #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mirounga
Copy link

Since the recurrent version of ntHash is a prefix sum with an associative operator, it is possible to compute the hashes in parallel using the prefix scan algorithm.
For 64-bit hash the theoretical speedup in 2x for AVX2 and 3x for AVX512.
Versions for 32-bit hashes are also included featuring even greater speedup.

@sjackman
Copy link
Collaborator

Hi, @mirounga. Cool! Does this code implement ntHash v1 or ntHash v2?

@mirounga
Copy link
Author

The code implements the current version of ntHash.
I have used the scalar version for validation

@sjackman
Copy link
Collaborator

Great. Thanks for confirming, @mirounga.

@mohamadi
Copy link
Collaborator

Hi @mirounga

I'm getting the following runtime performance results:

[hmohamadi@hpcg01 ntHash_avx]$ ./nttest_avx -k50 ERR294494_1.part1.fastq 
CPU time (sec) for hash algorithms for kmer=50
nthash	ntavx2	ntavx232	ntavx512	ntavx532	
6.72	2.14	2.25	6.84	6.98

The proper AVX512 and AVX512x32 implementations are not included in nttest_avx.cpp:

void hashSeqAvx512(const string & seq) {
        uint64_t fhVal, rhVal, hVal;
        hVal = NTC64(seq.c_str(), opt::kmerLen, fhVal, rhVal);
        if (hVal)opt::nz++;
        for (size_t i = 1; i < seq.length() - opt::kmerLen + 1; i++) {
                hVal = NTC64(seq[i - 1], seq[i - 1 + opt::kmerLen], opt::kmerLen, fhVal, rhVal);
                if (hVal)opt::nz++;
        }
}

void hashSeqAvx512x32(const string & seq) {
        uint64_t fhVal, rhVal, hVal;
        hVal = NTC64(seq.c_str(), opt::kmerLen, fhVal, rhVal);
        if (hVal)opt::nz++;
        for (size_t i = 1; i < seq.length() - opt::kmerLen + 1; i++) {
                hVal = NTC64(seq[i - 1], seq[i - 1 + opt::kmerLen], opt::kmerLen, fhVal, rhVal);
                if (hVal)opt::nz++;
        }
}

@mirounga
Copy link
Author

Hi @mohamadi ,
fixed the AVX512 functions.
When measuring performance please subtract the cost of IO - it is dominating now.

@parham-k
Copy link
Member

parham-k commented Sep 3, 2022

Closing due to being related to ntHash's old codebase. New ports are welcome!

@parham-k parham-k closed this Sep 3, 2022

kmerSeq += 3;

size_t sentinel = seq.length() - opt::kmerLen;
Copy link

@rchikhi rchikhi Oct 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expression should read seq.length() - opt::kmerLen + 1 otherwise last character isn't hashed and final hash doesn't match the scalar version. When this patch it applied, it does!

@rchikhi
Copy link

rchikhi commented Oct 8, 2022

Nice work there! I've expanded this unmerged PR into its own repository, in order to better assess the correctness of results, (keeping the same codebase): https://github.com/rchikhi/ntHash-AVX512

@mohamadi
Copy link
Collaborator

@parham-k can you take a look at it and merge if compatible with the new release?

@mohamadi mohamadi reopened this Oct 11, 2022
@parham-k
Copy link
Member

Sure! Thank you @rchikhi.

@parham-k parham-k self-assigned this Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants