-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tool to detect CRAM base corruption caused by GATK issue 8768 #8819
Conversation
Github actions tests reported job failures from actions build 8992751464
|
Github actions tests reported job failures from actions build 9063387853
|
Github actions tests reported job failures from actions build 9065623255
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detector tool and example tool output look good to me. I had only minor comments, which I will address myself in a followup commit in this branch.
* A diagnostic tool that analyzes a CRAM input file to look for conditions that trigger issue 8768. | ||
* | ||
* Writes a report to a file indicating whether the file appears to have read base corruption caused by 8768. | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a 1-2 paragraph description of the bug here, including under what circumstances it gets triggered, how it manifests, and what versions of GATK/Picard it affects. We should also post this information in a comment to issue 8768.
@ExperimentalFeature | ||
@WorkflowProperties | ||
@CommandLineProgramProperties( | ||
summary = "Analyze a CRAM file for issue 8768", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Analyze a CRAM file to check for base corruption caused by issue 8768"
@WorkflowProperties | ||
@CommandLineProgramProperties( | ||
summary = "Analyze a CRAM file for issue 8768", | ||
oneLineSummary = "Analyze a CRAM file for issue 8768", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Analyze a CRAM file to check for base corruption caused by issue 8768"
|
||
@Argument(fullName = StandardArgumentDefinitions.INPUT_LONG_NAME, | ||
shortName = StandardArgumentDefinitions.INPUT_SHORT_NAME, | ||
doc = "Input path of file to analyze", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file -> CRAM file
System.out.println(String.format("\nOutput written to %s\n", outputFile)); | ||
BAMIndexer.createAndWriteIndex(inputPath.toPath().toFile(), outputFile, true); | ||
System.out.println(String.format("\nOutput written to %s\n", outputPath)); | ||
// note this method is not nio aware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably note this limitation in the class-level docs as well
@@ -0,0 +1,126 @@ | |||
package org.broadinstitute.hellbender; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should be in the tools
package
@@ -81,6 +81,7 @@ public void testFileToFile(String fileIn, String extOut, String reference) throw | |||
@DataProvider(name="testingData") | |||
public Object[][] testingData() { | |||
return new String[][]{ | |||
// {"/Users/cnorman/projects/gatk/ultMerge.testfix.cram", ".cram", "/Users/cnorman/projects/gatk/Homo_sapiens_assembly19_1000genomes_decoy.fasta"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this
No description provided.