From 80b3eb85faf49d4c2758ad06b4391d67e8e98804 Mon Sep 17 00:00:00 2001
From: hanna <hanna@348d0f76-0448-11de-a6fe-93d51630548a>
Date: Fri, 18 Dec 2009 22:52:44 +0000
Subject: [PATCH] Fixed curiously epic failure in read-backed pileup: size()
 mismatched the numReads-numDeletions at that locus in the case where
 includeReadsWithDeletionsAtLoci == false, causing failures including bad
 output from pileup walker.  Also fixed up ValidatingPileup to run with the
 new ReadBackedPileup instead of just compiling successfully.

git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@2409 348d0f76-0448-11de-a6fe-93d51630548a
---
 .../gatk/iterators/LocusIteratorByState.java  |  3 ++-
 .../sting/gatk/refdata/SAMPileupRecord.java   | 21 +++++++++++++++++--
 .../walkers/qc/ValidatingPileupWalker.java    |  2 +-
 .../walkers/PileupWalkerIntegrationTest.java  |  2 +-
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/java/src/org/broadinstitute/sting/gatk/iterators/LocusIteratorByState.java b/java/src/org/broadinstitute/sting/gatk/iterators/LocusIteratorByState.java
index bb0d9b8dd..bfb6781fe 100755
--- a/java/src/org/broadinstitute/sting/gatk/iterators/LocusIteratorByState.java
+++ b/java/src/org/broadinstitute/sting/gatk/iterators/LocusIteratorByState.java
@@ -213,11 +213,12 @@ public class LocusIteratorByState extends LocusIterator {
 
             // todo -- performance problem -- should be lazy, really
             for ( SAMRecordState state : readStates ) {
-                size++;
                 if ( state.getCurrentCigarOperator() != CigarOperator.D && state.getCurrentCigarOperator() != CigarOperator.N ) {
+                    size++;
                     PileupElement p = new PileupElement(state.getRead(), state.getReadOffset());
                     pile.add(p);
                 } else if ( readInfo.includeReadsWithDeletionAtLoci() && state.getCurrentCigarOperator() != CigarOperator.N ) {
+                    size++;
                     pile.add(new PileupElement(state.getRead(), -1));
                     nDeletions++;
                 }
diff --git a/java/src/org/broadinstitute/sting/gatk/refdata/SAMPileupRecord.java b/java/src/org/broadinstitute/sting/gatk/refdata/SAMPileupRecord.java
index 60b4bd429..a84d9268a 100644
--- a/java/src/org/broadinstitute/sting/gatk/refdata/SAMPileupRecord.java
+++ b/java/src/org/broadinstitute/sting/gatk/refdata/SAMPileupRecord.java
@@ -9,6 +9,7 @@ import java.util.regex.Matcher;
 import org.broadinstitute.sting.utils.*;
 
 import net.sf.picard.reference.ReferenceSequenceFileWalker;
+import net.sf.samtools.util.StringUtil;
 
 /**
  *  This class wraps Maq/samtools allele calls from pileup format and presents them as a ROD.<br>
@@ -402,8 +403,24 @@ public class SAMPileupRecord implements Genotype, GenotypeList {
 
     public ArrayList<Byte> getBasesAsArrayList()        { throw new StingException("Not implemented"); }
     public ArrayList<Byte> getQualsAsArrayList()        { throw new StingException("Not implemented"); }
-    public byte[] getBases()        { throw new StingException("Not implemented"); }
-    public byte[] getQuals()        { throw new StingException("Not implemented"); }
+
+    /**
+     * Gets the bases in byte array form.
+     * @return byte array of the available bases.
+     */
+    public byte[] getBases() {
+        return StringUtil.stringToBytes(getBasesAsString());
+    }
+
+    /**
+     * Gets the Phred base qualities without ASCII offset.
+     * @return Phred base qualities.
+     */
+    public byte[] getQuals() {
+        byte[] quals = StringUtil.stringToBytes(getQualsAsString());
+        for(int i = 0; i < quals.length; i++) quals[i] -= 33;
+        return quals;
+    }
 
     /** Returns bases in the reference allele as a String. For point genotypes, the string consists of a single
      * character (reference base). For indel genotypes, the string is empty for insertions into
diff --git a/java/src/org/broadinstitute/sting/gatk/walkers/qc/ValidatingPileupWalker.java b/java/src/org/broadinstitute/sting/gatk/walkers/qc/ValidatingPileupWalker.java
index 0afab418d..01fdb90b7 100755
--- a/java/src/org/broadinstitute/sting/gatk/walkers/qc/ValidatingPileupWalker.java
+++ b/java/src/org/broadinstitute/sting/gatk/walkers/qc/ValidatingPileupWalker.java
@@ -70,7 +70,7 @@ public class ValidatingPileupWalker extends LocusWalker<Integer, ValidationStats
             return "Bases not equal";
 
         String aQuals = maybeSorted(new String(a.getQuals()), ! orderDependent );
-        String bQuals = maybeSorted(b.getQualsAsString(), ! orderDependent );
+        String bQuals = maybeSorted(new String(b.getQuals()), ! orderDependent );
         if ( ! aQuals.equals(bQuals) )
             return "Quals not equal";
 
diff --git a/java/test/org/broadinstitute/sting/gatk/walkers/PileupWalkerIntegrationTest.java b/java/test/org/broadinstitute/sting/gatk/walkers/PileupWalkerIntegrationTest.java
index 54e755c24..e6c0b4719 100644
--- a/java/test/org/broadinstitute/sting/gatk/walkers/PileupWalkerIntegrationTest.java
+++ b/java/test/org/broadinstitute/sting/gatk/walkers/PileupWalkerIntegrationTest.java
@@ -19,7 +19,7 @@ public class PileupWalkerIntegrationTest extends WalkerTest {
         String gatk_args = "-T Pileup -I /humgen/gsa-scr1/GATK_Data/Validation_Data/FHS_Pileup_Test.bam "
                  + "-R /seq/references/Homo_sapiens_assembly18/v0/Homo_sapiens_assembly18.fasta"
                  +  " -L chr15:46,347,148 -o %s";
-        String expected_md5 = "59edd722be96402be9dacd4c6f0b0c5e";
+        String expected_md5 = "d23032d10111755ccb1c1b01e6e097a7";
         WalkerTestSpec spec = new WalkerTestSpec(gatk_args, 1, Arrays.asList(expected_md5));
         executeTest("Testing the standard (no-indel) pileup on three merged FHS pools with 27 deletions in 969 bases", spec);
     }