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

Small error in publication #8

Open
christian-lanius opened this issue Jul 26, 2021 · 1 comment
Open

Small error in publication #8

christian-lanius opened this issue Jul 26, 2021 · 1 comment

Comments

@christian-lanius
Copy link

Hi everyone,
I have read with great interest your paper on this topic and while studying the paper in detail, I stumbled upon what I believe to be a small mistake in the printed paper: The initial value for the score is not -1, but rather initialized to qspan.
In the testbed, this is defined here: https://github.com/UCLA-VAST/minimap2-acceleration/blob/master/testbed/chain.c#L48
In the current master (of minimap2), this is also true.
I am really bad at reading HLS, but the same is done in the HLS code as well: https://github.com/UCLA-VAST/minimap2-acceleration/blob/master/kernel/hls/src/device_kernel.cpp#L218

In contrast, in figure 10, the score is initialized to -1, probably was just copied from the predecessor register chain, where this does hold true.
Again, this is purely a small mistake I encountered when reading the paper, but I thought I would mention it, in case somebody else stumbles upon it.

Kind regards
Christian

@dotkrnl
Copy link
Member

dotkrnl commented Jul 26, 2021

Hi Christian,

Thank you for your interest in our work!

You are correct. The initialization of score to q_span, or w[i] as in Equation (1) in our paper, is the implementation of score(i) = max{ ..., w[i] }. We missed this in Fig. 10, because it could be implemented as either the last stage or the initialization stage. We were preoccupied with the implementation of max{score[j] + weight(j,i)} when drawing the figure.

Thanks again for pointing this out! It will definitely help others who got confused by our mistake.

Thanks,
Jason

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

No branches or pull requests

2 participants