ReferenceConfidenceModel and tests #1442
Conversation
|
@akiezun There are several classes in here that I think should not be ported -- can we discuss in person when you have a free moment? |
|
yes On Mon, Jan 25, 2016 at 10:49 AM, droazen [email protected] wrote:
|
|
@droazen can you review? It should be straightforward. Let's keep the downsamplers for now and we'll fix this later |
|
@vruano and I agreed that he will review everything in the |
| /** | ||
| * AD field value for ref / non-ref | ||
| */ | ||
| private final int[] AD_Ref_Any = new int[2]; |
There was a problem hiding this comment.
Change field name to something more standard like simply AD.
There was a problem hiding this comment.
Actually I would split it in to int variables. It seems that get_AD_Ref_Any() bellow is used in such a restricted manner that is better just to create a in[] array on demand down there rather that maintain the 2 position array as a field. So:
private int refDepth = 0;
private int nonRefDepth = 0;
| final AlignmentContext alignmentContext = li.next(); | ||
| final ReadPileup p = alignmentContext.getBasePileup(); | ||
| Assert.assertTrue(p.size() == 1); | ||
| // TODO -- fix tests |
There was a problem hiding this comment.
Another TODO -- fix tests. Can we fix this now?
There was a problem hiding this comment.
these just work now
|
Review complete! Back to @akiezun |
|
as discussed with @droazen, I'm not touching this PR until he's done with his experiment. Reassigning to make it clear that I'm not working on this. |
|
I've backed up the state of these HC branches and have started the process of merging them with my engine work into a runnable tool. Feel free to address code review comments whenever you like, as it won't affect my work now. Back to @akiezun |
|
done with review. back to @droazen |
|
@droazen I added FractionalDownsampler now in a separate commit |
|
@akiezun If you've addressed all my comments, please merge this -- it does not require a second-pass review, and I could use the |
|
As discussed with @akiezun, I'll attempt the rebase and merge of this myself. |
droazen
force-pushed
the
ak_refConfModel
branch
from
db08354
to
246118b
Compare
|
Rebase complete -- waiting for tests |
ReferenceConfidenceModel and tests
@vruano please review
fixes #733