Changed filtering of normal hets on overlap with copy-ratio intervals… #4510
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
|
@sooheelee Are you comfortable reviewing this? The code changes are minimal. Note that there is no "regression test" since the test files do not give rise to the scenario you encountered in https://github.com/broadinstitute/dsde-docs/issues/2891. Perhaps we can make sure that we include such a test case when we address #4007. |
Codecov Report
@@ Coverage Diff @@
## master #4510 +/- ##
==============================================
- Coverage 79.121% 79.12% -0.001%
- Complexity 16677 16682 +5
==============================================
Files 1051 1051
Lines 60285 60293 +8
Branches 9875 9876 +1
==============================================
+ Hits 47698 47704 +6
- Misses 8759 8761 +2
Partials 3828 3828
|
|
I'm only in an introductory Java class, geared towards Android app development. So I cannot comment on the code. What I have done is take the branch and test it against the data that I have and I can say the counts now match up to the lower expected value. Furthermore, the four questionable sites are now absent. Before: With changes: Let me know if and how I can help with your efforts in creating a test if you decide to test for such a scenario. |
sooheelee
approved these changes
Mar 8, 2018
|
OK, thanks! @MartonKN, can you take a quick look at the code? |
… in ModelSegments to be consistent with filtering of case hets.
samuelklee
force-pushed
the
sl_filter_normal_hets
branch
from
27f75f7 to
76b34e2
Compare
MartonKN
reviewed
Mar 14, 2018
| hetAllelicCounts = new AllelicCountCollection( | ||
| metadata, | ||
| filteredAllelicCounts.getRecords().stream() | ||
| .filter(ac -> hetNormalAllelicCountSites.contains(ac.getInterval())) | ||
| .filter(ac -> hetNormalAllelicCounts.getOverlapDetector().overlapsAny(ac)) | ||
| .collect(Collectors.toList())); | ||
| final File hetAllelicCountsFile = new File(outputDir, outputPrefix + HET_ALLELIC_COUNTS_FILE_SUFFIX); | ||
| hetAllelicCounts.write(hetAllelicCountsFile); |
| public OverlapDetector<RECORD> getOverlapDetector() { | ||
| return OverlapDetector.create(getRecords()); | ||
| return overlapDetector.get(); | ||
| } | ||
|
|
||
| public Comparator<Locatable> getComparator() { |
| @@ -67,7 +67,7 @@ public AlleleFractionSegmentCollection findSegmentation(final int maxNumChangepo | |||
| "Log-linear factor for the penalty on the number of changepoints per chromosome must be non-negative."); | |||
|
|
|||
| logger.info(String.format("Finding changepoints in %d data points and %d chromosomes...", | |||
| allelicCounts.getRecords().size(), allelicCountsPerChromosome.size())); | |||
| allelicCounts.size(), allelicCountsPerChromosome.size())); | |||
|
|
|||
| //loop over chromosomes, find changepoints, and create allele-fraction segments | |||
| final List<AlleleFractionSegment> segments = new ArrayList<>(); | |||
| @@ -70,7 +70,7 @@ public CopyRatioSegmentCollection findSegmentation(final int maxNumChangepointsP | |||
| "Log-linear factor for the penalty on the number of changepoints per chromosome must be non-negative."); | |||
|
|
|||
| logger.info(String.format("Finding changepoints in %d data points and %d chromosomes...", | |||
| denoisedCopyRatios.getRecords().size(), denoisedCopyRatiosPerChromosome.size())); | |||
| denoisedCopyRatios.size(), denoisedCopyRatiosPerChromosome.size())); | |||
|
|
|||
| //loop over chromosomes, find changepoints, and create copy-ratio segments | |||
| final List<CopyRatioSegment> segments = new ArrayList<>(); | |||
| @@ -143,7 +143,7 @@ public MultidimensionalSegmentCollection findSegmentation(final int maxNumChange | |||
| kernelVarianceCopyRatio, kernelVarianceAlleleFraction, kernelScalingAlleleFraction); | |||
|
|
|||
| logger.info(String.format("Finding changepoints in (%d, %d) data points and %d chromosomes...", | |||
| denoisedCopyRatios.getRecords().size(), allelicCounts.size(), multidimensionalPointsPerChromosome.size())); | |||
| denoisedCopyRatios.size(), allelicCounts.size(), multidimensionalPointsPerChromosome.size())); | |||
|
|
|||
| //loop over chromosomes, find changepoints, and create allele-fraction segments | |||
| final List<MultidimensionalSegment> segments = new ArrayList<>(); | |||
| EXPECTED_METADATA.getSequenceDictionary(), hetNormalAllelicCounts.getMetadata().getSequenceDictionary())); | ||
| } | ||
| if (isAllelicCountsPresent && isNormalAllelicCountsPresent) { | ||
| Assert.assertEquals(hetAllelicCounts.getIntervals(), hetNormalAllelicCounts.getIntervals()); | ||
| } | ||
| } |
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.
… in ModelSegments to be consistent with filtering of case hets.
Also some miscellaneous code cleanup.