Overhaul tests on SV assembly-based non-complex breakpoint and type inference code #4835

SHuang-Broad · 2018-05-31T17:58:21Z

This is to overhaul tests on SV assembly-based non-complex breakpoint and type inference code.

What is done:

new AssemblyBasedSVDiscoveryTestDataProvider classes which hold manually computed expected values. Unit tests simply load the expected values and compare with actual values calculated on the fly
fix bugs in BND formatted variants and tested (using the structure above).

Classes affected in main

AnnotatedVariantProducer: methods are grouped together and renamed to reflect that the annotations added are assembly-specific or using short reads
BreakEndVariantType: more detailed types (mostly about how alt allele, with the ref bases and square brackets)
BreakpointComplications and BreakpointsInference: mostly to add trivial methods used only in tests
DiscoverVariantsFromContigAlignmentsSAMSpark: now a thin CLI, where functionalities are refactored into new class ContigChimericAlignmentIterativeInterpreter
SimpleChimera: new documented and tested method firstContigRegionRefSpanAfterSecond and trivial test-related code

cwhelan

Here are some comments for you, mostly focused on the refactoring and other code changes. I haven't been able to go through all of the tests in great detail but the few I've looked at seemed to make sense. Thanks for adding all these tests.

cwhelan · 2018-06-01T14:07:48Z

...in/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducer.java


        final VariantContextBuilder builder0 = new VariantContextBuilder(firstVar);
        builder0.attribute(linkKey, secondVar.getID());

        final VariantContextBuilder builder1 = new VariantContextBuilder(secondVar);
        builder1.attribute(linkKey, firstVar.getID());

+


extra whitespace

cwhelan · 2018-06-04T14:37:50Z

...in/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducer.java

+                                                             final Broadcast<SAMSequenceDictionary> broadcastSequenceDictionary,
+                                                             final Broadcast<SVIntervalTree<VariantContext>> broadcastCNVCalls,
+                                                             final String sampleId) throws IOException {
+        if ( inferredType instanceof BreakEndVariantType ) {


Since we're doing a lot of switching on the inferred type here and below on line 179, as a refactoring suggestion what if you moved this method to SVType subclasses? It seems like a good candidate for it.

cwhelan · 2018-06-04T14:41:46Z

...in/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducer.java

+                                                              final Broadcast<SAMSequenceDictionary> broadcastSequenceDictionary,
+                                                              final Broadcast<SVIntervalTree<VariantContext>> broadcastCNVCalls,
+                                                              final String sampleId) {
+        if (broadcastCNVCalls == null || end < 0)


Why would end be less than zero here. That seems like a strange condition that should be caught upstream somewhere.

I'm still not sure about this end < 0 check, do you know why this is here?

EDIT
Sorry I cannot reply (it's "outdated"....), so I edited your message.
I forgot to remove them. Previously it was to guard against taking in an end of an BND record which up to that point (with previous implementations) is negative (end was later converted to simply take value of POS to populate the VariantContextBuider.stop(end); both previous and current implementation uses POS for the value of stop in constructing the VC)

Is this referring to SvTYPE.NO_APPLICABLE_END? If so please reference that. Also, what if we added methods hasApplicableEnd and hasApplicableLength to SvType and used those first instead of checking for negative numbers?

EDIT
Sorry I cannot reply (it's "outdated"....), so I edited your message.
Updated as suggested.

cwhelan · 2018-06-04T15:02:01Z

src/main/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/BreakEndVariantType.java

-        private static boolean indicatesInv55(boolean bracketPointsLeft, boolean basesFirst) {
-            return (bracketPointsLeft && basesFirst);
+        @Override
+        public boolean equals(final Object o) {


You could remove these equals and hashcode methods if they are just calling super

cwhelan · 2018-06-04T15:02:41Z

src/main/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/BreakEndVariantType.java

+        }
+
+        @Override
+        public boolean equals(final Object o) {


again could remove these

cwhelan · 2018-06-06T17:08:30Z

...org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducerUnitTest.java

+        VariantContext actual = AnnotatedVariantProducer.produceAnnotatedVcFromAssemblyEvidence(del_chr20_28831535_29212083, simpleNovelAdjacencyAndChimericAlignmentEvidence, referenceBroadcast, refSeqDictBroadcast, null, "testSample").make();
+        VariantContextTestUtils.assertVariantContextsAreEqual(actual, expected, Collections.singletonList(HQ_MAPPINGS));
+
+        // cnv call available


Thanks for adding this test

cwhelan · 2018-06-06T17:23:05Z

.../hellbender/tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProvider.java

+
+        // these are used for testing
+        public final boolean expectedFirstContigRegionHasLaterReferenceMapping;
+        public final SimpleChimera manuallyCuratedSimpleChimera;


Can you describe what you mean by "manually curated"? Is it the just the expected values for these results? If so can you rename them just to "expected"?

cwhelan · 2018-06-06T17:27:05Z

.../hellbender/tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProvider.java

+            this.manuallyCuratedBiPathBubble = manuallyCuratedBiPathBubble;
+            this.manuallyCuratedSVTypes = manuallyCuratedSVTypes;
+            this.manuallyCuratedVariants = manuallyCuratedVariants;
+            this.inferencer = inferencer;


Why do you have to store the inferencer class along with the test data? Is there any way that you could make selecting the inferencer part of the test too? It seems like it allows the potentially confusing condition of writing a test but then choosing an inferencer that wouldn't actually be chosen by the system in an integration setting. Alternatively, why not just have different data providers for each inferencer subtype?

After talking in person, I think renaming it expectedInferencerClass better conveys the idea.

cwhelan · 2018-06-06T17:31:45Z

.../tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProviderForSimpleSV.java

+    public static final TestDataForSimpleSV forComplexTanDup_2to3_short_noPseudoHom_minus;
+
+    static {
+        try ( final ByteArrayOutputStream outputStream = new ByteArrayOutputStream() ){


Why is there a single outputStream shared by all the tests? It looks like most of the tests call reset on it before writing anything to it. Why have a shared one at all -- why not just do it in each test?

Also, do you need output streams at all? It looks like you are just using them to concatenate the portions of the chimera (flanks and homology). Why not just use string concatentation?

Was trying to avoid multiple try blocks. But using Strings removed the need for streams.

cwhelan · 2018-06-06T17:33:21Z

.../tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProviderForSimpleSV.java

+     * Return a list of two entries for positive and reverse strand representations.
+     */
+    private static List<TestDataForSimpleSV>
+    forSimpleDeletion(final ByteArrayOutputStream outputStream) throws IOException {


It would be nice in these tests if you could add a few comments to each one demarcating the input test data and the expected results. Otherwise it's hard to figure out where one stops and the other stops.

named all variables that are expected accordingly

codecov-io · 2018-06-11T21:50:40Z

Codecov Report

Merging #4835 into master will increase coverage by 0.359%.
The diff coverage is 75.255%.

@@               Coverage Diff               @@
##              master     #4835       +/-   ##
===============================================
+ Coverage     80.468%   80.828%   +0.359%     
- Complexity     17852     19207     +1355     
===============================================
  Files           1093      1091        -2     
  Lines          64295     68880     +4585     
  Branches       10378     11597     +1219     
===============================================
+ Hits           51737     55674     +3937     
- Misses          8504      8996      +492     
- Partials        4054      4210      +156

Impacted Files	Coverage Δ	Complexity Δ
.../sv/StructuralVariationDiscoveryPipelineSpark.java	`88.652% <ø> (ø)`	`13 <0> (ø)`	⬇️
...ignment/AssemblyContigWithFineTunedAlignments.java	`77.519% <ø> (+7.752%)`	`44 <0> (+6)`	⬆️
...tructuralVariationDiscoveryArgumentCollection.java	`97.222% <ø> (-0.075%)`	`0 <0> (ø)`
.../sv/discovery/inference/CpxVariantInterpreter.java	`78.814% <ø> (+10.431%)`	`25 <0> (ø)`	⬇️
...lignment/AssemblyContigAlignmentsConfigPicker.java	`92.937% <ø> (ø)`	`86 <0> (ø)`	⬇️
...iscoverFromLocalAssemblyContigAlignmentsSpark.java	`74.561% <0%> (-3.42%)`	`2 <0> (ø)`
...overy/inference/TypeInferredFromSimpleChimera.java	`100% <100%> (ø)`	`2 <2> (+1)`	⬆️
.../discovery/inference/ImpreciseVariantDetector.java	`81.25% <100%> (ø)`	`6 <0> (ø)`	⬇️
...nce/SegmentedCpxVariantSimpleVariantExtractor.java	`93.96% <100%> (ø)`	`71 <0> (ø)`	⬇️
...pleNovelAdjacencyAndChimericAlignmentEvidence.java	`87.5% <100%> (ø)`	`10 <1> (ø)`	⬇️
... and 113 more

SHuang-Broad

@cwhelan Hi Chris, I've incorporated most of the changes you suggested and separated them into 3 commits.
Would you take a look again when possible? Thanks!

SHuang-Broad · 2018-06-11T19:07:45Z

.../tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProviderForSimpleSV.java

+     * Return a list of two entries for positive and reverse strand representations.
+     */
+    private static List<TestDataForSimpleSV>
+    forSimpleDeletion(final ByteArrayOutputStream outputStream) throws IOException {


named all variables that are expected accordingly

SHuang-Broad · 2018-06-11T19:34:53Z

.../tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProviderForSimpleSV.java

+    public static final TestDataForSimpleSV forComplexTanDup_2to3_short_noPseudoHom_minus;
+
+    static {
+        try ( final ByteArrayOutputStream outputStream = new ByteArrayOutputStream() ){


Was trying to avoid multiple try blocks. But using Strings removed the need for streams.

SHuang-Broad · 2018-06-11T20:13:05Z

.../hellbender/tools/spark/sv/discovery/inference/AssemblyBasedSVDiscoveryTestDataProvider.java

+
+        // these are used for testing
+        public final boolean expectedFirstContigRegionHasLaterReferenceMapping;
+        public final SimpleChimera manuallyCuratedSimpleChimera;


SHuang-Broad · 2018-06-11T20:14:03Z

...org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducerUnitTest.java

 public class AnnotatedVariantProducerUnitTest extends GATKBaseTest {

    /**
     * Hack to force trigger test data generation.
     */
    @BeforeClass
    private void makeSureDataIsAvailable() {
-        if(!SimpleSVDiscoveryTestDataProvider.testDataInitialized) {
-            new SimpleSVDiscoveryTestDataProvider();
+        if(!AssemblyBasedSVDiscoveryTestDataProviderForSimpleSV.testDataInitialized) {


yep. removed

SHuang-Broad · 2018-06-11T20:15:22Z

...org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducerUnitTest.java

+                                            final List<VariantContext> expectedVariants) throws IOException {
+        if (inferredTypes.size() == 1) {
+            VariantContextTestUtils.assertVariantContextsAreEqual(produceAnnotatedVcFromAssemblyEvidence(inferredTypes.get(0), novelAdjacencyAndAssemblyEvidence, broadcastReference, broadcastSequenceDictionary, null, sampleId).make(),
+                    expectedVariants.get(0), Arrays.asList(CONTIG_NAMES, HQ_MAPPINGS)); // these two are omitted because: 1) HQ_MAPPINGS equal testing code is wrong (saying 1!=1), 2)CONTIG_NAMES will be tested in another test


I actually debugged on this,
if i recall correctly, it is saying an integer 1 not equal to an String "1" (note HQ_MAPPINGS holds a simple integer), which is a strange feature of VC. So I stopped there.
This attribute is later tested in testMiscCases()

SHuang-Broad · 2018-06-12T00:18:34Z

...adinstitute/hellbender/tools/spark/sv/discovery/inference/NovelAdjacencyAndAltHaplotype.java

    /**
     * @return the inferred type could be
     *          1) a single entry for simple variants, or
-     *          2) a list of two entries for "replacement" case where the ref- and alt- path are both >= {@link STRUCTURAL_VARIANT_SIZE_LOWER_BOUND} bp long, or
+     *          2) a list of two entries for "replacement" case where the ref- and alt- path are both >=
+     *             {@link StructuralVariationDiscoveryArgumentCollection#STRUCTURAL_VARIANT_SIZE_LOWER_BOUND} bp long, or
     *          3) a list of two entries with BND mates.
     *          It is safe, for now, to assume that ({@link SimpleSVType} and {@link BreakEndVariantType} are never mixed)
     */
    @VisibleForTesting
    public List<SvType> toSimpleOrBNDTypes(final ReferenceMultiSource reference, final SAMSequenceDictionary referenceDictionary) {

        switch (type) {


Note taken.

SHuang-Broad · 2018-06-12T00:29:25Z

...titute/hellbender/tools/spark/sv/discovery/DiscoverVariantsFromContigAlignmentsSAMSpark.java

-        List<VariantContext> annotatedVariants = discoverVariantsFromChimeras(svDiscoveryInputMetaData, parsedContigAlignments);
+        @SuppressWarnings("deprecation")
+        List<VariantContext> annotatedVariants =
+                org.broadinstitute.hellbender.tools.spark.sv.discovery.inference.ContigChimericAlignmentIterativeInterpreter


I need to suppress the deprecation warning.
So cannot import the class.
That's my understanding after reading
https://stackoverflow.com/a/20909204/2666587
The method in question is deprecated because it is the method that I'm trying to phase out, and used in three places

DiscoverVariantsFromContigAlignmentsSAMSpark: tool I'm planning to phase out

StructuralVariationDiscoveryPipelineSpark: before the tool is phased out, I need to call into it.

SegmentedCpxVariantSimpleVariantExtractor: for extracting simple variants from CPX calls (a working but really not beautiful solution)

SHuang-Broad · 2018-06-12T00:41:52Z

...titute/hellbender/tools/spark/sv/discovery/DiscoverVariantsFromContigAlignmentsSAMSpark.java

@@ -136,7 +120,7 @@ protected void runTool(final JavaSparkContext ctx) {
                StructuralVariationDiscoveryPipelineSpark.broadcastCNVCalls(ctx, getHeaderForReads(),
                        discoverStageArgs.cnvCallsFile);

-        final String vcfOutputFile = prefixForOutput + "_" + SVUtils.getSampleId(getHeaderForReads()) + "_inv_del_ins.vcf";
+        final String vcfOutputFile = prefixForOutput + (prefixForOutput.endsWith("/") ? "" : "_") + SVUtils.getSampleId(getHeaderForReads()) + "_inv_del_ins.vcf";


addressed with a new method getVcfOutputPath(), and did the same for SvDiscoverFromLocalAssemblyContigAlignmentsSpark

SHuang-Broad · 2018-06-12T00:48:38Z

src/main/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/BreakEndVariantType.java

+                return ins;
+            } else {
+                if (narl.getStrandSwitch() == StrandSwitch.NO_SWITCH) {
+                    return narl.getComplication().getInsertedSequenceForwardStrandRep();


using suggested change. Thanks!

SHuang-Broad · 2018-06-12T01:04:54Z

src/main/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/BreakEndVariantType.java

-        private final boolean indicatesInv55;
+        @VisibleForTesting
+        static String extractInsertedSequence(final NovelAdjacencyAndAltHaplotype narl, final boolean forUpstreamLoc) {
+            final String ins = narl.getComplication().getInsertedSequenceForwardStrandRep();


There are actually two issues at hand here,

We could have a reverse strand representation of the event; this is covered by BreakpointComplications.reverseComplementIfNecessary()

We could have a right/downstream BND record, to whom the inserted sequence (and homology as well), must be RC of the left/upstream BND record.
The second case is very confusing, and is the case handled here.
Picture this left breakpoint of an inversion
where ref is
GGGGGGG .......... TTTTTTTTT
and the alt is (+ strand rep)
GGGGGGGXXXXXXAAAAAAA.....
where XXXXX is inserted (assume its RC is Y), and (-) strand rep is
..........TTTTTTTTYYYYYYYCCCCCCC.
Unfortunately, this (-) strand rep is also the (+) strand representation of the downstream BND record.
And this holds for homology as well....... 🙀

cwhelan

Looks better @SHuang-Broad. A few more comments and questions.

cwhelan · 2018-06-13T17:43:35Z

...in/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducer.java

+
+        // manually remove inserted sequence information from RPL event-produced DEL, when it can be linked with an INS
+        if (linkedVariants._1 instanceof SimpleSVType.Deletion)
+            return Arrays.asList(builder1.rmAttribute(GATKSVVCFConstants.INSERTED_SEQUENCE).rmAttribute(GATKSVVCFConstants.INSERTED_SEQUENCE_LENGTH).rmAttribute(GATKSVVCFConstants.SEQ_ALT_HAPLOTYPE).make(),


Can you shorten these lines for readability?

cwhelan · 2018-06-13T17:49:15Z

...in/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/AnnotatedVariantProducer.java

+                                                              final Broadcast<SAMSequenceDictionary> broadcastSequenceDictionary,
+                                                              final Broadcast<SVIntervalTree<VariantContext>> broadcastCNVCalls,
+                                                              final String sampleId) {
+        if (broadcastCNVCalls == null || end < 0)


I'm still not sure about this end < 0 check, do you know why this is here?

EDIT
Sorry I cannot reply (it's "outdated"....), so I edited your message.
I forgot to remove them. Previously it was to guard against taking in an end of an BND record which up to that point (with previous implementations) is negative (end was later converted to simply take value of POS to populate the VariantContextBuider.stop(end); both previous and current implementation uses POS for the value of stop in constructing the VC)

cwhelan · 2018-06-13T17:52:58Z

src/main/java/org/broadinstitute/hellbender/tools/spark/sv/discovery/BreakEndVariantType.java

+        @VisibleForTesting
+        static String extractInsertedSequence(final NovelAdjacencyAndAltHaplotype narl, final boolean forUpstreamLoc) {
+            final String ins = narl.getComplication().getInsertedSequenceForwardStrandRep();
+            if (ins.isEmpty()) {


Does reverse complement thrown an exception if you give it an empty string? If not, you could skip this check and make this method a 2-liner.

no it doesn't, according to a unit test I wrote temporarily. So updated as suggested. Thanks!

cwhelan · 2018-06-13T17:59:55Z

...titute/hellbender/tools/spark/sv/discovery/DiscoverVariantsFromContigAlignmentsSAMSpark.java

-        List<VariantContext> annotatedVariants = discoverVariantsFromChimeras(svDiscoveryInputMetaData, parsedContigAlignments);
+        @SuppressWarnings("deprecation")
+        List<VariantContext> annotatedVariants =
+                org.broadinstitute.hellbender.tools.spark.sv.discovery.inference.ContigChimericAlignmentIterativeInterpreter


OK, but if we're still using it why bother deprecating it? I'd use the actual deprecation tag for parts of a public API or something, but here we don't really need to warn ourselves about it, do we? It just seems counterproductive to deprecate the method and then add warning suppressors everywhere we call it.

EDIT
Sorry I cannot reply (it's "outdated"....), so I edited your message.
Agree. So I removed the deprecated annotation on the method, and fixed the warning suppressions everywhere.

cwhelan · 2018-06-13T18:02:09Z

...titute/hellbender/tools/spark/sv/discovery/DiscoverVariantsFromContigAlignmentsSAMSpark.java

-        } finally {
-            narlsAndSources.unpersist();
+    private String getVcfOutputPath() {
+        if ( java.nio.file.Files.exists(Paths.get(prefixForOutput)) ) {


Can't you import this class?

cwhelan · 2018-06-13T19:47:07Z

...rg/broadinstitute/hellbender/tools/spark/sv/discovery/inference/BreakpointComplications.java

@@ -287,14 +301,49 @@ public boolean equals(Object obj) {
     * NOTE THAT this does not handle case when one of the duplication copy is inverted;
     * for that see {@link IntraChrStrandSwitchBreakpointComplications} (possibly partially).
     */
-    abstract static class SmallDuplicationBreakpointComplications extends BreakpointComplications {
+    abstract public static class SmallDuplicationBreakpointComplications extends BreakpointComplications {


Put public first in the keywords list

cwhelan · 2018-06-13T19:48:09Z

...rg/broadinstitute/hellbender/tools/spark/sv/discovery/inference/BreakpointComplications.java

+
+        IntraChrStrandSwitchBreakpointComplications(final SimpleChimera simpleChimera, final byte[] contigSeq, final boolean firstAfterSecond) {
+            resolveComplicationForSimpleStrandSwitch(simpleChimera, contigSeq, firstAfterSecond);
+//            if ( simpleChimera.isCandidateInvertedDuplication() ) {


remove commented code

cwhelan · 2018-06-13T19:52:41Z

...a/org/broadinstitute/hellbender/tools/spark/sv/discovery/inference/BreakpointsInference.java

@@ -343,7 +366,8 @@ void resolveComplications(final SimpleChimera simpleChimera, final byte[] contig
        protected BNDTypeBreakpointsInference(final SimpleChimera simpleChimera,
                                              final byte[] contigSequence,
                                              final SAMSequenceDictionary referenceDictionary) {
-            super(simpleChimera, contigSequence);
+            super(simpleChimera, contigSequence, referenceDictionary);
+            altHaplotypeSequence = new byte[]{};


But we do have the assembled haplotype.. It's true that it's somewhat redundant but I was thinking it might be nice to have there for consistency with the other variant types. I forget -- do we include any padding in the alt haplotype, or is it really just the inserted sequence in this case?

EDIT
Sorry I cannot reply (it's "outdated"....), so I edited your message.
No this doesn't pad around the breakpoints; I think Valentine does some padding when he does genotyping, but not here.
I made a last, optional commit that puts back this annotation for BND records, but I'm not sure how much of a value it contains.

cwhelan · 2018-06-13T19:58:06Z

...stitute/hellbender/tools/spark/sv/discovery/inference/CpxVariantCanonicalRepresentation.java

-            final boolean firstSegmentNeighborsHeadAlignment = basicInfo.forwardStrandRep ? (firstSegment.getStart() - head.referenceSpan.getEnd() == 1)
-                                                                                          : (head.referenceSpan.getStart() - firstSegment.getEnd() == 1);
-            if ( ! firstSegmentNeighborsHeadAlignment )
+            final boolean expectedCase = basicInfo.forwardStrandRep ? (firstSegment.getStart() - head.referenceSpan.getEnd() == 1)


The old name for this variable (firstSegmentNeighborsHeadAlignment) seemed more descriptive.

reverted back

cwhelan · 2018-06-13T19:59:15Z

...adinstitute/hellbender/tools/spark/sv/discovery/inference/NovelAdjacencyAndAltHaplotype.java

+        if (isCandidateForFatInsertion()) {
+            return new SimpleInterval(leftJustifiedLeftRefLoc.getContig(), leftJustifiedLeftRefLoc.getStart(), leftJustifiedRightRefLoc.getEnd());
+        } else
+            throw new UnsupportedOperationException("trying to get interval from a novel adjacency that is no a fat insertion: " + toString());


cwhelan

👍

Looks good apart from the optional commit which I think we should hold back to be its own unit of work as discussed in person.

… inference code Also * expand cases on BND format variants * change to how VariantContext are extracted from NARL and SVType * refactor the tool DiscoverVariantsFromContigAlignmentsSAMSpark into a new worker class ContigChimericAlignmentIterativeInterpreter

… inference code (#4835) Also * expand cases on BND format variants * change to how VariantContext are extracted from NARL and SVType * refactor the tool DiscoverVariantsFromContigAlignmentsSAMSpark into a new worker class ContigChimericAlignmentIterativeInterpreter

SHuang-Broad added enhancement tests SV labels May 31, 2018

cwhelan requested changes Jun 6, 2018

View reviewed changes

SHuang-Broad commented Jun 12, 2018

View reviewed changes

cwhelan requested changes Jun 13, 2018

View reviewed changes

cwhelan approved these changes Jun 20, 2018

View reviewed changes

SHuang-Broad force-pushed the sh-sv-assembly-based-sv-inference-test-overhaul branch 4 times, most recently from b0008d8 to aee9931 Compare June 22, 2018 17:05

SHuang-Broad force-pushed the sh-sv-assembly-based-sv-inference-test-overhaul branch from aee9931 to 7c291ba Compare June 23, 2018 00:21

SHuang-Broad merged commit 6a3dd94 into master Jun 23, 2018

SHuang-Broad deleted the sh-sv-assembly-based-sv-inference-test-overhaul branch June 23, 2018 05:11

Overhaul tests on SV assembly-based non-complex breakpoint and type inference code #4835

Overhaul tests on SV assembly-based non-complex breakpoint and type inference code #4835

Conversation

SHuang-Broad commented May 31, 2018

cwhelan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhelan Jun 13, 2018 • edited by SHuang-Broad Loading

Choose a reason for hiding this comment

cwhelan Jun 13, 2018 • edited by SHuang-Broad Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jun 11, 2018 • edited Loading

Codecov Report

SHuang-Broad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhelan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhelan Jun 13, 2018 • edited by SHuang-Broad Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhelan Jun 13, 2018 • edited by SHuang-Broad Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhelan Jun 13, 2018 • edited by SHuang-Broad Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhelan left a comment

Choose a reason for hiding this comment

cwhelan Jun 13, 2018 •

edited by SHuang-Broad

Loading

cwhelan Jun 13, 2018 •

edited by SHuang-Broad

Loading

codecov-io commented Jun 11, 2018 •

edited

Loading

cwhelan Jun 13, 2018 •

edited by SHuang-Broad

Loading

cwhelan Jun 13, 2018 •

edited by SHuang-Broad

Loading

cwhelan Jun 13, 2018 •

edited by SHuang-Broad

Loading