Updated plotting for ModelSegments CNV pipeline. #3729

samuelklee · 2017-10-20T18:42:18Z

This is rebased off of #3716, since it depends on code there. Hence, only the second commit needs to be reviewed in this PR.

The code and tests are quite similar to that for PlotSegmentedCopyRatio/PlotACNVResults. However, I've changed the R scripts to be more efficient (WGS plots no longer take several hours). Furthermore, PlotModeledSegments is more flexible than PlotACNVResults in that it plots CR, AF, or both on the fly depending on the available inputs. I've also added some more input validation, changed some terminology, and moved over to data.table for reading TSVs in R.

samuelklee · 2017-10-20T18:48:24Z

src/main/java/org/broadinstitute/hellbender/tools/copynumber/plotting/PlottingUtils.java

+/**
+ * @author Samuel Lee &lt;slee@broadinstitute.org&gt;
+ */
+final class PlottingUtils {


Note to self: add parameter validation to the methods in this class, just for consistency.

codecov-io · 2017-10-20T19:44:47Z

Codecov Report

Merging #3729 into master will decrease coverage by 0.011%.
The diff coverage is 75.294%.

@@               Coverage Diff               @@
##              master     #3729       +/-   ##
===============================================
- Coverage     79.642%   79.632%   -0.011%     
- Complexity     17608     17872      +264     
===============================================
  Files           1138      1146        +8     
  Lines          63529     64546     +1017     
  Branches        9693      9882      +189     
===============================================
+ Hits           50596     51399      +803     
- Misses          9057      9251      +194     
- Partials        3876      3896       +20

Impacted Files	Coverage Δ	Complexity Δ
...copynumber/formats/CopyNumberStandardArgument.java	`0% <0%> (ø)`	`0 <0> (?)`
...ls/copynumber/plotting/PlotDenoisedCopyRatios.java	`100% <100%> (ø)`	`6 <6> (?)`
...formats/collections/SampleLocatableCollection.java	`95.652% <100%> (+0.652%)`	`9 <2> (+2)`	⬆️
...tools/copynumber/coverage/copyratio/CopyRatio.java	`34.783% <34.783%> (ø)`	`4 <4> (?)`
...ynumber/multidimensional/model/ModeledSegment.java	`56.25% <56.25%> (ø)`	`6 <6> (?)`
...number/coverage/copyratio/CopyRatioCollection.java	`61.538% <61.538%> (ø)`	`3 <3> (?)`
...ltidimensional/model/ModeledSegmentCollection.java	`69.767% <69.767%> (ø)`	`3 <3> (?)`
...ender/tools/copynumber/plotting/PlottingUtils.java	`80% <80%> (ø)`	`11 <11> (?)`
...tools/copynumber/plotting/PlotModeledSegments.java	`93.443% <93.443%> (ø)`	`23 <23> (?)`
...discovery/prototype/SuspectedTransLocDetector.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
... and 21 more

samuelklee · 2017-10-23T12:17:57Z

@LeeTL1220 Do you mind reviewing the second commit? Most of it consists of lightly edited code that is already in master, so hopefully the review should be quick.

LeeTL1220

Comments are pretty minor. Also, a question or two...

LeeTL1220 · 2017-10-23T12:59:05Z

...ain/java/org/broadinstitute/hellbender/tools/copynumber/plotting/PlotDenoisedCopyRatios.java

+    private void checkRegularReadableUserFiles() {
+        Utils.nonNull(outputPrefix);
+        IOUtils.canReadFile(inputStandardizedCopyRatiosFile);
+        IOUtils.canReadFile(inputDenoisedCopyRatiosFile);


Does this work if the input file is not specified?

(the input files are optional)

Whoops, they should not be.

LeeTL1220 · 2017-10-23T13:00:04Z

src/main/java/org/broadinstitute/hellbender/tools/copynumber/plotting/PlotModeledSegments.java

+ * <h3>Example</h3>
+ *
+ * <pre>
+ * gatk-launch --javaOptions "-Xmx4g" PlotACNVResults \


IS this command line correct? PlotACNVResults?

Good catch.

I also need to update the rest of this javadoc.

Updated the javadoc for PlotDenoisedCopyRatios as well.

LeeTL1220 · 2017-10-23T13:03:54Z

...oadinstitute/hellbender/tools/copynumber/plotting/PlotDenoisedCopyRatiosIntegrationTest.java

+        Assert.assertTrue(new File(outputDir, OUTPUT_PREFIX + ".deltaMAD.txt").length() > 0);
+        Assert.assertTrue(new File(outputDir, OUTPUT_PREFIX + ".scaledDeltaMAD.txt").exists());
+        Assert.assertTrue(new File(outputDir, OUTPUT_PREFIX + ".scaledDeltaMAD.txt").length() > 0);
+        final double standardizedMAD = ParamUtils.readValuesFromFile(new File(outputDir, OUTPUT_PREFIX + ".standardizedMAD.txt"))[0];


Are these MAD numbers akin to the old qc metric?

They are exactly the same. I figured that changing the name of the metric to be descriptive/informative couldn't hurt. Let me know which of these files we can do away with, if any (do we really need both scaleDeltaMAD and scaledMAD, for example)?

It really does make some of our collaborator's lives easier to have all of those....

No problem. I think it might make sense to move the emission of these to DenoiseReadCounts, eventually.

LeeTL1220 · 2017-10-23T13:04:43Z

...oadinstitute/hellbender/tools/copynumber/plotting/PlotDenoisedCopyRatiosIntegrationTest.java

@@ -0,0 +1,167 @@
+package org.broadinstitute.hellbender.tools.copynumber.plotting;


I'm assuming that you have looked at the plots themselves to ensure that they are okay...

Great... ignore this comment then.

samuelklee · 2017-10-23T13:09:22Z

...n/resources/org/broadinstitute/hellbender/tools/copynumber/plotting/PlotDenoisedCopyRatios.R

+    write.table(round((standardizedMAD - denoisedMAD) / standardizedMAD, 3), file.path(output_dir, paste(output_prefix, ".scaledDeltaMAD.txt", sep="")), col.names=FALSE, row.names=FALSE)
+
+    #plot standardized and denoised copy ratio on top of each other
+    pre_color_blue="#3B5DFF"


Note to self, white space.

samuelklee · 2017-10-23T13:28:22Z

Will merge once @asmirnov239 reviews #3716.

LeeTL1220 · 2017-10-23T15:19:29Z

@samuelklee Looks good to me.

samuelklee commented Oct 20, 2017

View reviewed changes

samuelklee requested a review from LeeTL1220 October 23, 2017 12:15

samuelklee mentioned this pull request Oct 23, 2017

Ported CallSegments as CallCopyRatioSegments for ModelSegments CNV pipeline. #3731

Merged

LeeTL1220 reviewed Oct 23, 2017

View reviewed changes

LeeTL1220 assigned samuelklee Oct 23, 2017

samuelklee commented Oct 23, 2017

View reviewed changes

samuelklee added 2 commits October 25, 2017 11:25

Updated plotting for ModelSegments CNV pipeline.

5de623f

Addressed PR comments.

81bb7d5

samuelklee force-pushed the sl_plotting branch from 09fb1bf to 1328be4 Compare October 25, 2017 15:28

Rebased on merged sl_collections.

8c22183

samuelklee force-pushed the sl_plotting branch from 1328be4 to 8c22183 Compare October 25, 2017 18:02

samuelklee merged commit 12e212d into master Oct 25, 2017

samuelklee deleted the sl_plotting branch October 25, 2017 19:08

This was referenced Nov 9, 2017

Speed up CNV plotting for WGS. #3554

Closed

Add AAF plot output to PlotACNVResults. #2957

Closed

Use only log_2 or real space for copy ratios. #2886

Closed

jonn-smith pushed a commit that referenced this pull request Nov 27, 2017

Updated plotting for ModelSegments CNV pipeline. (#3729)

066d972

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated plotting for ModelSegments CNV pipeline. #3729

Updated plotting for ModelSegments CNV pipeline. #3729

samuelklee commented Oct 20, 2017

samuelklee Oct 20, 2017

codecov-io commented Oct 20, 2017 •

edited

Loading

samuelklee commented Oct 23, 2017

LeeTL1220 left a comment

LeeTL1220 Oct 23, 2017

LeeTL1220 Oct 23, 2017

samuelklee Oct 23, 2017

LeeTL1220 Oct 23, 2017

samuelklee Oct 23, 2017

samuelklee Oct 23, 2017

samuelklee Oct 23, 2017

LeeTL1220 Oct 23, 2017

samuelklee Oct 23, 2017

LeeTL1220 Oct 23, 2017

samuelklee Oct 23, 2017

LeeTL1220 Oct 23, 2017

samuelklee Oct 23, 2017

LeeTL1220 Oct 23, 2017

samuelklee Oct 23, 2017

samuelklee commented Oct 23, 2017

LeeTL1220 commented Oct 23, 2017

		@@ -0,0 +1,167 @@
		package org.broadinstitute.hellbender.tools.copynumber.plotting;

Updated plotting for ModelSegments CNV pipeline. #3729

Updated plotting for ModelSegments CNV pipeline. #3729

Conversation

samuelklee commented Oct 20, 2017

Choose a reason for hiding this comment

codecov-io commented Oct 20, 2017 • edited Loading

Codecov Report

samuelklee commented Oct 23, 2017

LeeTL1220 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

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

samuelklee commented Oct 23, 2017

LeeTL1220 commented Oct 23, 2017

codecov-io commented Oct 20, 2017 •

edited

Loading