UCLA-VAST / minimap2-acceleration

Hardware Acceleration of Long Read Pairwise Overlapping in Genome Sequencing: Open Source Repository
http://vast.cs.ucla.edu/sites/default/files/publications/minimap2-acc-approved.pdf
MIT License
31 stars 12 forks source link

Small error in publication #8

Open christian-lanius opened 3 years ago

christian-lanius commented 3 years ago

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 commented 3 years ago

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