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

PairHMM stripe initialization of M_t_1_y is wrong #163

Open
philipc opened this issue Dec 12, 2021 · 1 comment
Open

PairHMM stripe initialization of M_t_1_y is wrong #163

philipc opened this issue Dec 12, 2021 · 1 comment

Comments

@philipc
Copy link

philipc commented Dec 12, 2021

See this code:
https://github.com/Intel-HLS/GKL/blob/51580c453003bc60e082fa8aaafc97457e0f6057/src/main/native/pairhmm/avx-pairhmm-template.h#L201

M_t_1_y should not be an exact copy of M_t_1, but instead it should be a copy prior to the vector shift. That means it should be initialized to VEC_SET1_VAL(zero) at the start of the stripe.

Background:
I have ported this code to Rust (https://github.com/philipc/gkl-rs), and I have been investigating why the values differ from the scalar implementation. In addition to this bug, there is also a difference due to use of approximations for the match-to-match probability. After addressing both of these, the Rust implementation produces values that very closely match the expected results in https://github.com/Intel-HLS/GKL/blob/master/src/test/resources/pairhmm-testdata.txt

@Kmannth
Copy link
Contributor

Kmannth commented Dec 13, 2021

Thanks for this note we will look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants