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

AVX present? #5291

Merged
merged 7 commits into from
Nov 19, 2018
+28 −27
Merged

AVX present? #5291

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2af3c00
Check if AVX available & suggest remedy if not.
EdwardDixon Nov 12, 2018
59ee071
Trivial commit just to force a build
EdwardDixon Nov 13, 2018
91bb5c0
Removing env we don't need
EdwardDixon Nov 14, 2018
ac40183
Removed references to the separate environment for Intel enhancements
EdwardDixon Nov 14, 2018
1d75954
Equality!
EdwardDixon Nov 15, 2018
014211d
Pulling command name into static string.
EdwardDixon Nov 16, 2018
a1e13fc
Fixing accidental deletion
EdwardDixon Nov 19, 2018
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
21 changes: 19 additions & 2 deletions src/main/java/org/broadinstitute/hellbender/tools/walkers/vqsr/CNNScoreVariants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
import org.broadinstitute.hellbender.utils.runtime.AsynchronousStreamWriter;
import org.broadinstitute.hellbender.utils.variant.GATKVCFConstants;
import org.broadinstitute.hellbender.utils.variant.GATKVCFHeaderLines;
import org.broadinstitute.hellbender.exceptions.UserException;
import picard.cmdline.programgroups.VariantFilteringProgramGroup;

import com.intel.gkl.IntelGKLUtils;

import java.io.*;
import java.util.*;

Expand Down Expand Up @@ -108,6 +111,9 @@ public class CNNScoreVariants extends TwoPassVariantWalker {
"1D models will look at the reference sequence and variant annotations." +
"2D models look at aligned reads, reference sequence, and variant annotations." +
"2D models require a BAM file as input as well as the tensor-type argument to be set.";
static final String AVXREQUIRED_ERROR = "This tool requires AVX instruction set support by default due to its dependency on recent versions of the TensorFlow library.\n" +
" If you have an older (pre-1.6) version of TensorFlow installed that does not require AVX you may attempt to re-run the tool with the --disable-avx-check argument to bypass this check.\n" +
" Note that such configurations are not officially supported.";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a string constant DISABLE_AVX_CHECK_NAME for the value disable-avx-check, and replace the argument name thats embedded in this String with a %s format specifier. When the exception is thrown below, call call String.format passing it this string and the DISABLE_AVX_CHECK_NAME string.


private static final int CONTIG_INDEX = 0;
private static final int POS_INDEX = 1;
Expand Down Expand Up @@ -160,6 +166,11 @@ public class CNNScoreVariants extends TwoPassVariantWalker {
@Argument(fullName = "output-tensor-dir", shortName = "output-tensor-dir", doc = "Optional directory where tensors can be saved for debugging or visualization.", optional = true)
private String outputTensorsDir = "";

@Advanced
@Argument(fullName = "disable-avx-check", shortName = "disable-avx-check", doc = "If set, no check will be made for AVX support. " +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fullName and shortName should both reference the DISABLE_AVX_CHECK_NAME constant mentioned above.

"Use only if you have installed a pre-1.6 TensorFlow build. ", optional = true)
private boolean disableAVXCheck = false;

@Hidden
@Argument(fullName = "enable-journal", shortName = "enable-journal", doc = "Enable streaming process journal.", optional = true)
private boolean enableJournal = false;
Expand Down Expand Up @@ -232,8 +243,14 @@ public List<ReadFilter> getDefaultReadFilters() {

@Override
public void onTraversalStart() {
if (getHeaderForVariants().getGenotypeSamples().size() > 1) {
logger.warn("CNNScoreVariants is a single sample tool, but the input VCF has more than 1 sample.");
// Users can disable the AVX check to allow an older version of TF that doesn't require AVX to be used.
if(this.disableAVXCheck == false) {
IntelGKLUtils utils = new IntelGKLUtils();
utils.load(null);
if (utils.isAvxSupported() == false) {
// Give user the bad news, suggest remedies.
throw new UserException.HardwareFeatureException(CNNScoreVariants.AVXREQUIRED_ERROR);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw new UserException.HardwareFeatureException(String.format(CNNScoreVariants.AVXREQUIRED_ERROR, DISABLE_AVX_CHECK_NAME);

}

// Start the Python process and initialize a stream writer for streaming data to the Python code
Expand Down