Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Changed exact-match tests for ModelSegments to allow for numerical differences due to Java version. #8111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Changed exact-match tests for ModelSegments to allow for numerical differences due to Java version. #8111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bff7df
Changed exact-match tests for ModelSegments to allow for numerical di…
samuelklee Nov 29, 2022
bc9b5a7
Unpinned Java 11 version for testing.
samuelklee Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/gatk-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ jobs:
needs: check-secrets
strategy:
matrix:
java: [ 8, 11.0.11+9 ]
java: [ 8, 11 ]
experimental: [ false ]
scalaVersion: [ 2.11, 2.12 ]
testType: [ cloud, integration, unit ]
exclude:
- java: 11.0.11+9
- java: 11
scalaVersion: 2.11
- java: 8
scalaVersion: 2.12
Expand Down
45 changes: 28 additions & 17 deletions ...est/java/org/broadinstitute/hellbender/tools/copynumber/ModelSegmentsIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.broadinstitute.hellbender.tools.copynumber;

import htsjdk.samtools.SAMException;
import htsjdk.samtools.util.IOUtil;
import org.apache.hadoop.util.ComparableVersion;
import org.broadinstitute.hellbender.CommandLineProgramTest;
import org.broadinstitute.hellbender.testutils.ArgumentsBuilder;
import org.broadinstitute.hellbender.testutils.CopyNumberTestUtils;
Expand All @@ -22,12 +21,9 @@
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

/**
* Integration tests for {@link ModelSegments}. We test for input validation across various run modes of the tool
Expand All @@ -53,6 +49,7 @@ public void assertThatExpectedOutputUpdateToggleIsDisabled() {

private static final File TEST_SUB_DIR = new File(toolsTestDir, "copynumber");
private static final File LARGE_TEST_RESOURCES_SUB_DIR = new File(largeFileTestDir, "org/broadinstitute/hellbender/tools/copynumber");
private static final File EXACT_MATCH_EXPECTED_SUB_DIR_LEGACY = new File(TEST_SUB_DIR, "model-segments-expected/legacy");
private static final File EXACT_MATCH_EXPECTED_SUB_DIR = new File(TEST_SUB_DIR, "model-segments-expected");
private static final File TUMOR_1_DENOISED_COPY_RATIOS_FILE = new File(LARGE_TEST_RESOURCES_SUB_DIR,
"model-segments-wes-tumor-1-denoised-copy-ratios-SM-74P4M-v1-chr20.denoisedCR.tsv");
Expand Down Expand Up @@ -184,18 +181,29 @@ public Object[][] dataInvalidDataModesSingleSample() {
};
}

/**
* We will check for different expected results depending on the Java version to account for numerical differences between versions.
* See https://github.com/broadinstitute/gatk/issues/8107.
* We assume that 11.0.11 is the delineating version based on information in that thread, but this has not been checked in detail.
*/
private static boolean isLegacyRuntimeVersion() {
final ComparableVersion version = new ComparableVersion(System.getProperty("java.version"));
return version.compareTo(new ComparableVersion("11.0.11")) <= 0; // should also be valid for checking pre-Java 9 version strings (e.g. "1.8")
}

@Test(dataProvider = "dataValidDataModesSingleSample")
public void testValidDataModesSingleSample(final String outputPrefix,
final File denoisedCopyRatiosFile,
final File allelicCountsFile,
final File normalAllelicCountsFile) {
final File outputDir = UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS ? EXACT_MATCH_EXPECTED_SUB_DIR : createTempDir("testDir");
final File expectedDir = isLegacyRuntimeVersion() ? EXACT_MATCH_EXPECTED_SUB_DIR_LEGACY : EXACT_MATCH_EXPECTED_SUB_DIR;
final File outputDir = UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS ? expectedDir : createTempDir("testDir");
final ArgumentsBuilder argsBuilder = buildArgsBuilderSingleSample(
outputDir, outputPrefix, denoisedCopyRatiosFile, allelicCountsFile, normalAllelicCountsFile);
runCommandLine(argsBuilder);
final boolean isAllelicCountsPresent = allelicCountsFile != null;
final boolean isNormalAllelicCountsPresent = normalAllelicCountsFile != null;
assertOutputFilesSingleSample(outputDir, outputPrefix, isAllelicCountsPresent, isNormalAllelicCountsPresent);
assertOutputFilesSingleSample(outputDir, outputPrefix, expectedDir, isAllelicCountsPresent, isNormalAllelicCountsPresent);
}

@Test(dataProvider = "dataInvalidDataModesSingleSample", expectedExceptions = IllegalArgumentException.class)
Expand Down Expand Up @@ -236,6 +244,7 @@ private static ArgumentsBuilder buildArgsBuilderSingleSample(final File outputDi
*/
private static void assertOutputFilesSingleSample(final File outputDir,
final String outputPrefix,
final File expectedDir,
final boolean isAllelicCountsPresent,
final boolean isNormalAllelicCountsPresent) {
Assert.assertFalse(!isAllelicCountsPresent && isNormalAllelicCountsPresent);
Expand All @@ -247,7 +256,7 @@ private static void assertOutputFilesSingleSample(final File outputDir,
ModelSegments.ALLELE_FRACTION_MODEL_PARAMETER_FILE_SUFFIX)) {
CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + fileTag + suffix),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + fileTag + suffix),
new File(expectedDir, outputPrefix + fileTag + suffix),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);
}
Expand All @@ -268,7 +277,7 @@ private static void assertOutputFilesSingleSample(final File outputDir,

CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + ModelSegments.COPY_RATIO_SEGMENTS_FOR_CALLER_FILE_SUFFIX),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + ModelSegments.COPY_RATIO_SEGMENTS_FOR_CALLER_FILE_SUFFIX),
new File(expectedDir, outputPrefix + ModelSegments.COPY_RATIO_SEGMENTS_FOR_CALLER_FILE_SUFFIX),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);
final CopyRatioSegmentCollection copyRatioSegments = new CopyRatioSegmentCollection(
Expand All @@ -277,20 +286,20 @@ private static void assertOutputFilesSingleSample(final File outputDir,

CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + ModelSegments.COPY_RATIO_LEGACY_SEGMENTS_FILE_SUFFIX),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + ModelSegments.COPY_RATIO_LEGACY_SEGMENTS_FILE_SUFFIX),
new File(expectedDir, outputPrefix + ModelSegments.COPY_RATIO_LEGACY_SEGMENTS_FILE_SUFFIX),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);
CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + ModelSegments.ALLELE_FRACTION_LEGACY_SEGMENTS_FILE_SUFFIX),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + ModelSegments.ALLELE_FRACTION_LEGACY_SEGMENTS_FILE_SUFFIX),
new File(expectedDir, outputPrefix + ModelSegments.ALLELE_FRACTION_LEGACY_SEGMENTS_FILE_SUFFIX),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);

AllelicCountCollection hetAllelicCounts = null;
if (isAllelicCountsPresent) {
CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + ModelSegments.HET_ALLELIC_COUNTS_FILE_SUFFIX),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + ModelSegments.HET_ALLELIC_COUNTS_FILE_SUFFIX),
new File(expectedDir, outputPrefix + ModelSegments.HET_ALLELIC_COUNTS_FILE_SUFFIX),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);
hetAllelicCounts = new AllelicCountCollection(
Expand All @@ -300,7 +309,7 @@ private static void assertOutputFilesSingleSample(final File outputDir,
if (isNormalAllelicCountsPresent) { //if this is true, case sample allelic counts will be present
CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + ModelSegments.NORMAL_HET_ALLELIC_COUNTS_FILE_SUFFIX),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + ModelSegments.NORMAL_HET_ALLELIC_COUNTS_FILE_SUFFIX),
new File(expectedDir, outputPrefix + ModelSegments.NORMAL_HET_ALLELIC_COUNTS_FILE_SUFFIX),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);
final AllelicCountCollection hetNormalAllelicCounts = new AllelicCountCollection(
Expand Down Expand Up @@ -447,15 +456,16 @@ public void testValidDataModesMultipleSamples(final String outputPrefix,
final List<File> denoisedCopyRatiosFiles,
final List<File> allelicCountsFiles,
final File normalAllelicCountsFile) {
final File outputDir = UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS ? EXACT_MATCH_EXPECTED_SUB_DIR : createTempDir("testDir");
final File expectedDir = isLegacyRuntimeVersion() ? EXACT_MATCH_EXPECTED_SUB_DIR_LEGACY : EXACT_MATCH_EXPECTED_SUB_DIR;
final File outputDir = UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS ? expectedDir : createTempDir("testDir");

// test joint segmentation
final ArgumentsBuilder argsBuilder = buildArgsBuilderMultipleSamples(
outputDir, outputPrefix, denoisedCopyRatiosFiles, allelicCountsFiles, normalAllelicCountsFile);
runCommandLine(argsBuilder);
final boolean isAllelicCountsPresent = allelicCountsFiles != null;
final boolean isNormalAllelicCountsPresent = normalAllelicCountsFile != null;
assertOutputFilesMultipleSamples(outputDir, outputPrefix, isAllelicCountsPresent, isNormalAllelicCountsPresent);
assertOutputFilesMultipleSamples(outputDir, outputPrefix, expectedDir, isAllelicCountsPresent, isNormalAllelicCountsPresent);

// test using joint segmentation as input to scatter of first case sample
final ArgumentsBuilder argsBuilderSingleSample = buildArgsBuilderSingleSample(
Expand All @@ -467,7 +477,7 @@ public void testValidDataModesMultipleSamples(final String outputPrefix,
CopyNumberStandardArgument.SEGMENTS_FILE_LONG_NAME,
new File(outputDir, outputPrefix + ModelSegments.PICARD_INTERVAL_LIST_FILE_SUFFIX));
runCommandLine(argsBuilderSingleSample);
assertOutputFilesSingleSample(outputDir, outputPrefix + "-tumor-1", isAllelicCountsPresent, isNormalAllelicCountsPresent);
assertOutputFilesSingleSample(outputDir, outputPrefix + "-tumor-1", expectedDir, isAllelicCountsPresent, isNormalAllelicCountsPresent);
}

@Test(dataProvider = "dataInvalidDataModesMultipleSamples", expectedExceptions = IllegalArgumentException.class)
Expand Down Expand Up @@ -504,6 +514,7 @@ private static ArgumentsBuilder buildArgsBuilderMultipleSamples(final File outpu

private static void assertOutputFilesMultipleSamples(final File outputDir,
final String outputPrefix,
final File expectedDir,
final boolean isAllelicCountsPresent,
final boolean isNormalAllelicCountsPresent) {
Assert.assertFalse(!isAllelicCountsPresent && isNormalAllelicCountsPresent);
Expand All @@ -523,7 +534,7 @@ private static void assertOutputFilesMultipleSamples(final File outputDir,

CopyNumberTestUtils.assertFilesEqualUpToAllowedDeltaForDoubleValues(
new File(outputDir, outputPrefix + ModelSegments.PICARD_INTERVAL_LIST_FILE_SUFFIX),
new File(EXACT_MATCH_EXPECTED_SUB_DIR, outputPrefix + ModelSegments.PICARD_INTERVAL_LIST_FILE_SUFFIX),
new File(expectedDir, outputPrefix + ModelSegments.PICARD_INTERVAL_LIST_FILE_SUFFIX),
ALLOWED_DELTA_FOR_DOUBLE_VALUES,
logger);
}
Expand Down
2 changes: 2 additions & 0 deletions ...tools/copynumber/model-segments-expected/legacy/multiple-sample-ac-nac-tumor-1.af.igv.seg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Sample Chromosome Start End Num_Probes Segment_Mean
SM-74P4M-1 20 138125 62871232 827 0.372342
2 changes: 2 additions & 0 deletions ...tools/copynumber/model-segments-expected/legacy/multiple-sample-ac-nac-tumor-1.cr.igv.seg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Sample Chromosome Start End Num_Probes Segment_Mean
SM-74P4M-1 20 138125 62871232 0 NaN
5 changes: 5 additions & 0 deletions ...der/tools/copynumber/model-segments-expected/legacy/multiple-sample-ac-nac-tumor-1.cr.seg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@HD VN:1.6
@SQ SN:20 LN:63025520 UR:http://www.broadinstitute.org/ftp/pub/seq/references/Homo_sapiens_assembly19.fasta AS:GRCh37 M5:0dec9660ec1efaaf33281c0d5ea2560f SP:Homo Sapiens
@RG ID:GATKCopyNumber SM:SM-74P4M-1
CONTIG START END NUM_POINTS_COPY_RATIO MEAN_LOG2_COPY_RATIO
20 138125 62871232 0 NaN
Loading