HOLD: Bring in more duplication expansion calls #3668
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #3668 +/- ##
==============================================
+ Coverage 78.463% 79.444% +0.98%
- Complexity 16421 17539 +1118
==============================================
Files 1041 1137 +96
Lines 59099 63304 +4205
Branches 9673 9668 -5
==============================================
+ Hits 46371 50291 +3920
- Misses 8985 9157 +172
- Partials 3743 3856 +113
|
SHuang-Broad
force-pushed
the
sh_insdel_containment_bug
branch
2 times, most recently
from
dcfd714 to
1cd756b
Compare
SHuang-Broad
force-pushed
the
sh_insdel_containment_bug
branch
from
1cd756b to
7d20771
Compare
…e. dup); refactored some related test code
magicDGS
reviewed
Nov 2, 2017
| import static org.broadinstitute.hellbender.tools.spark.sv.utils.GATKSVVCFConstants.*; | ||
|
|
||
|
|
||
| public class SimpleSvTypeInferenceAndConstructionUnitTest extends BaseTest { |
There was a problem hiding this comment.
After #3475, this should extend GATKBaseTest
SHuang-Broad
changed the title
SHuang-Broad
force-pushed
the
sh_insdel_containment_bug
branch
from
7d20771 to
90c133b
Compare
SHuang-Broad
commented
Jan 23, 2018
| @@ -131,8 +133,8 @@ SimpleInterval getInvertedTransInsertionRefSpan() { | |||
| return invertedTransInsertionRefSpan; | |||
| } | |||
|
|
|||
| boolean hasDupSeqButNoStrandSwitch() { | |||
| return hasDuplicationAnnotation && dupSeqStrandOnCtg.stream().noneMatch(s -> s.equals(Strand.NEGATIVE)); | |||
| boolean indicatesInvDup() { | |||
There was a problem hiding this comment.
to be picked up 1.
| private void resolveComplicationForSimpleTandupExpansion(final SimpleInterval leftReferenceInterval, | ||
| final SimpleInterval rightReferenceInterval, | ||
| final AlignmentInterval firstContigRegion, | ||
| private void resolveComplicationForSimpleTandupExpansion(final AlignmentInterval firstContigRegion, |
There was a problem hiding this comment.
to be picked up 3
| @@ -303,6 +305,7 @@ private void initForInvDup(final ChimericAlignment chimericAlignment, final byte | |||
| */ | |||
| @VisibleForTesting | |||
| public static boolean isLikelyInvertedDuplication(final AlignmentInterval one, final AlignmentInterval two) { | |||
| if (one.forwardStrand==two.forwardStrand) return false; | |||
There was a problem hiding this comment.
to be picked up 2.
| @@ -328,15 +331,15 @@ private void initForInsDel(final ChimericAlignment chimericAlignment, final byte | |||
|
|
|||
| final boolean oneContainedInTheOther = leftReferenceSpan.contains(rightReferenceSpan) || rightReferenceSpan.contains(leftReferenceSpan); | |||
| if (oneContainedInTheOther) { | |||
| resolveComplicationForSimpleTandupExpansion(leftReferenceSpan, rightReferenceSpan, firstContigRegion, secondContigRegion, r1e, r2b, distBetweenAlignRegionsOnCtg, contigSeq, true); | |||
| } else if ( distBetweenAlignRegionsOnRef > 0 ) { // Deletion: | |||
| resolveComplicationForSimpleTandupExpansion(firstContigRegion, secondContigRegion, r1e, r2b, contigSeq); | |||
There was a problem hiding this comment.
to be picked up 4.1
| resolveComplicationForSimpleDel(firstContigRegion, secondContigRegion, distBetweenAlignRegionsOnCtg, contigSeq); | ||
| } else if (distBetweenAlignRegionsOnRef == 0 && distBetweenAlignRegionsOnCtg > 0) { // Insertion: simple insertion, inserted sequence is the sequence [c1e+1, c2b-1] on the contig | ||
| insertedSequenceForwardStrandRep = getInsertedSequence(firstContigRegion, secondContigRegion, contigSeq); | ||
| } else if (distBetweenAlignRegionsOnRef == 0 && distBetweenAlignRegionsOnCtg < 0) { // Tandem repeat contraction: reference has two copies but one copy was deleted on the contig; duplicated sequence on reference are [r1e-|d2|+1, r1e] and [r2b, r2b+|d2|-1] | ||
| resolveComplicationForSimpleTandupContraction(leftReferenceSpan, firstContigRegion, secondContigRegion, r1e, c1e, c2b, contigSeq); | ||
| } else if (distBetweenAlignRegionsOnRef < 0 && distBetweenAlignRegionsOnCtg >= 0) { // Tandem repeat expansion: reference bases [r1e-|d1|+1, r1e] to contig bases [c1e-|d1|+1, c1e] and [c2b, c2b+|d1|-1] with optional inserted sequence [c1e+1, c2b-1] in between the two intervals on contig | ||
| resolveComplicationForSimpleTandupExpansion(leftReferenceSpan, rightReferenceSpan, firstContigRegion, secondContigRegion, r1e, r2b, distBetweenAlignRegionsOnCtg, contigSeq, false); | ||
| resolveComplicationForSimpleTandupExpansion(firstContigRegion, secondContigRegion, r1e, r2b, contigSeq); |
There was a problem hiding this comment.
to be picked up 4.2
| @@ -386,6 +425,85 @@ private void resolveComplicationForSimpleTandupExpansion(final SimpleInterval le | |||
| } | |||
| } | |||
|
|
|||
| static byte[] extractAltHaplotypeForTandupExpansionWithContainment(final AlignmentInterval firstContigRegion, | |||
There was a problem hiding this comment.
to be picked up 6
| determineStrandSwitch(current, next), involvesRefPositionSwitch(current, next)); | ||
| if (isNotSimpleTranslocation) | ||
| results.add(new ChimericAlignment(current, next, insertionMappings, alignedContig.contigName)); | ||
| final ChimericAlignment chimericAlignment = new ChimericAlignment(current, next, insertionMappings, alignedContig.contigName); |
There was a problem hiding this comment.
method to be phased out, but it makes sense to let down stream users to check what type of novel adjacency this CA could point to, which requests the state checkers to be more comprehensive
| public static boolean nextAlignmentMayBeInsertion(final AlignmentInterval current, final AlignmentInterval next, | ||
| final Integer minAlignLength, final int mapQThresholdInclusive, | ||
| final boolean filterWhollyContained) { | ||
| static boolean nextAlignmentMayBeInsertion(final AlignmentInterval current, final AlignmentInterval next, |
There was a problem hiding this comment.
check how this is different from the new method splitPairStrongEnoughEvidenceForCA()
| if (involvesRefPositionSwitch(regionWithLowerCoordOnContig, regionWithHigherCoordOnContig)) | ||
| return new Tuple2<>(regionWithHigherCoordOnContig.referenceSpan, regionWithLowerCoordOnContig.referenceSpan); | ||
| else | ||
| if (isForwardStrandRepresentation) { |
There was a problem hiding this comment.
used in two places: BC.iniForInsDel() and NARL.leftJustifyBreakpoints().
The first use is trivial as it is type specific, whereas for the 2nd use in NARL, we need to be careful since we are considering expanding the NARL to cover cases of ref order switch.
| @@ -61,60 +61,57 @@ protected NovelAdjacencyReferenceLocations(final Kryo kryo, final Input input) { | |||
| @VisibleForTesting | |||
| static Tuple2<SimpleInterval, SimpleInterval> leftJustifyBreakpoints(final ChimericAlignment ca, | |||
There was a problem hiding this comment.
to be picked up.
|
closing since the changes are already in master, and in a branch about to generate a new PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dealing with case when two alignment blocks contain each other in their ref span, indicating duplication.
Instead of outputting CIGARs for the duplicated units like we did for duplication records now in master pipeline, we output alt haplotype sequence from the evidence contigs we assembled, since we did an experiment work with inverted duplications.
Some performance evaluation based on the logs
No variants that were dropped are simple variants, and they are expected to be brought back with the correct interpretation once complex sv PR series are fully coded.
Two known issues: