this.allele, getAlleles(), and getAltAlleles() now return List not set

-- Changes associated code throughout the codebase
-- Updated necessary (but minimal) UnitTests to reflect new behavior
-- Much better makealleles() function in VC.java that enforces a lot of key constraints in VC

public/java/src/org/broadinstitute/sting/gatk/walkers/annotator/AlleleBalanceBySample.java

@@ -47,7 +47,7 @@ public class AlleleBalanceBySample extends GenotypeAnnotation implements Experim
         if (!g.isHet())
             return null;
 
-        Set<Allele> altAlleles = vc.getAlternateAlleles();
+        Collection<Allele> altAlleles = vc.getAlternateAlleles();
         if ( altAlleles.size() == 0 )
             return null;
 

public/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/AlleleFrequencyCalculationModel.java

@@ -33,6 +33,7 @@ import org.broadinstitute.sting.utils.variantcontext.Genotype;
 import org.broadinstitute.sting.utils.variantcontext.VariantContext;
 
 import java.io.PrintStream;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -76,7 +77,7 @@ public abstract class AlleleFrequencyCalculationModel implements Cloneable {
      */
     protected abstract void getLog10PNonRef(RefMetaDataTracker tracker,
                                             ReferenceContext ref,
-                                            Map<String, Genotype> GLs,  Set<Allele> Alleles,
+                                            Map<String, Genotype> GLs,  List<Allele> Alleles,
                                             double[] log10AlleleFrequencyPriors,
                                             double[] log10AlleleFrequencyPosteriors);
 

public/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/ExactAFCalculationModel.java

@@ -29,19 +29,14 @@ import org.apache.log4j.Logger;
 import org.broadinstitute.sting.gatk.contexts.ReferenceContext;
 import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker;
 import org.broadinstitute.sting.utils.MathUtils;
-import org.broadinstitute.sting.utils.SimpleTimer;
 import org.broadinstitute.sting.utils.Utils;
 import org.broadinstitute.sting.utils.exceptions.UserException;
 import org.broadinstitute.sting.utils.variantcontext.Allele;
 import org.broadinstitute.sting.utils.variantcontext.Genotype;
 import org.broadinstitute.sting.utils.variantcontext.VariantContext;
-import sun.reflect.generics.reflectiveObjects.NotImplementedException;
 
 import java.io.PrintStream;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
 
 public class ExactAFCalculationModel extends AlleleFrequencyCalculationModel {
     //
@@ -58,7 +53,7 @@ public class ExactAFCalculationModel extends AlleleFrequencyCalculationModel {
 
     public void getLog10PNonRef(RefMetaDataTracker tracker,
                                 ReferenceContext ref,
-                                Map<String, Genotype> GLs, Set<Allele>alleles,
+                                Map<String, Genotype> GLs, List<Allele> alleles,
                                 double[] log10AlleleFrequencyPriors,
                                 double[] log10AlleleFrequencyPosteriors) {
         final int numAlleles = alleles.size();

public/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/GridSearchAFEstimation.java

@@ -54,7 +54,7 @@ public class GridSearchAFEstimation extends AlleleFrequencyCalculationModel {
 
     protected void getLog10PNonRef(RefMetaDataTracker tracker,
                                    ReferenceContext ref,
-                                   Map<String, Genotype> GLs, Set<Allele>alleles,
+                                   Map<String, Genotype> GLs, List<Allele> alleles,
                                    double[] log10AlleleFrequencyPriors,
                                    double[] log10AlleleFrequencyPosteriors) {
         initializeAFMatrix(GLs);

public/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedGenotyperEngine.java

@@ -423,7 +423,7 @@ public class UnifiedGenotyperEngine {
 
         int endLoc = calculateEndPos(vc.getAlleles(), vc.getReference(), loc);
 
-        Set<Allele> myAlleles = vc.getAlleles();
+        Set<Allele> myAlleles = new HashSet<Allele>(vc.getAlleles());
         // strip out the alternate allele if it's a ref call
         if ( bestAFguess == 0 && UAC.GenotypingMode == GenotypeLikelihoodsCalculationModel.GENOTYPING_MODE.DISCOVERY ) {
             myAlleles = new HashSet<Allele>(1);
@@ -447,7 +447,7 @@ public class UnifiedGenotyperEngine {
         return new VariantCallContext(vcCall, confidentlyCalled(phredScaledConfidence, PofF));
     }
 
-    private int calculateEndPos(Set<Allele> alleles, Allele refAllele, GenomeLoc loc) {
+    private int calculateEndPos(Collection<Allele> alleles, Allele refAllele, GenomeLoc loc) {
         // TODO - temp fix until we can deal with extended events properly
         // for indels, stop location is one more than ref allele length
         boolean isSNP = true, hasNullAltAllele = false;

public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/evaluators/ValidationReport.java

@@ -10,6 +10,7 @@ import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
 import org.broadinstitute.sting.utils.variantcontext.Allele;
 import org.broadinstitute.sting.utils.variantcontext.VariantContext;
 
+import java.util.Collection;
 import java.util.Set;
 
 /**
@@ -142,8 +143,8 @@ public class ValidationReport extends VariantEvaluator implements StandardEval {
 
 
     public boolean haveDifferentAltAlleles(VariantContext eval, VariantContext comp) {
-        Set<Allele> evalAlts = eval.getAlternateAlleles();
+        Collection<Allele> evalAlts = eval.getAlternateAlleles();
-        Set<Allele> compAlts = comp.getAlternateAlleles();
+        Collection<Allele> compAlts = comp.getAlternateAlleles();
         if ( evalAlts.size() != compAlts.size() ) {
             return true;
         } else {

public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantValidationAssessor.java

@@ -237,7 +237,7 @@ public class VariantValidationAssessor extends RodWalker<VariantContext,Integer>
         infoMap.put("HomVarPct", String.format("%.1f", 100.0*homVarProp));
         infoMap.put("HetPct", String.format("%.1f", 100.0*hetProp));
         infoMap.put("HW", String.format("%.2f", hwScore));
-        Set<Allele> altAlleles = vContext.getAlternateAlleles();
+        Collection<Allele> altAlleles = vContext.getAlternateAlleles();
         int altAlleleCount = altAlleles.size() == 0 ? 0 : vContext.getChromosomeCount(altAlleles.iterator().next());
         if ( !isViolation && altAlleleCount > 0 )
             numTrueVariants++;

public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContext.java

@@ -181,7 +181,7 @@ public class VariantContext implements Feature { // to enable tribble intergrati
     protected Type type = null;
 
     /** A set of the alleles segregating in this context */
-    protected LinkedHashSet<Allele> alleles = null;
+    final protected List<Allele> alleles;
 
     /** A mapping from sampleName -> genotype objects for all genotypes associated with this context */
     protected Map<String, Genotype> genotypes = null;
@@ -355,7 +355,7 @@ public class VariantContext implements Feature { // to enable tribble intergrati
         if ( alleles == null ) { throw new IllegalArgumentException("Alleles cannot be null"); }
 
         // we need to make this a LinkedHashSet in case the user prefers a given ordering of alleles
-        this.alleles = alleleCollectionToSet(new LinkedHashSet<Allele>(), alleles);
+        this.alleles = makeAlleles(alleles);
 
 
         if ( genotypes == null ) { genotypes = NO_GENOTYPES; }
@@ -445,7 +445,7 @@ public class VariantContext implements Feature { // to enable tribble intergrati
      * @param alleles the set of allele segregating alleles at this site.  Must include those in genotypes, but may be more
      * @return vc subcontext
      */
-    public VariantContext subContextFromGenotypes(Collection<Genotype> genotypes, Set<Allele> alleles) {
+    public VariantContext subContextFromGenotypes(Collection<Genotype> genotypes, Collection<Allele> alleles) {
         return new VariantContext(getSource(), contig, start, stop, alleles, genotypes != null ? genotypeCollectionToMap(new TreeMap<String, Genotype>(), genotypes) : null, getNegLog10PError(), filtersWereApplied() ? getFilters() : null, getAttributes(), getReferenceBaseForIndel());
     }
 
@@ -687,17 +687,6 @@ public class VariantContext implements Feature { // to enable tribble intergrati
         return ref;
     }
 
-    /** Private helper routine that grabs the reference allele but doesn't throw an error if there's no such allele */
-
-//    private Allele getReferenceWithoutError() {
-//        for ( Allele allele : getAlleles() ) {
-//            if ( allele.isReference() ) {
-//                return allele;
-//            }
-//        }
-//
-//        return null;
-//    }
 
     /**
      * @return true if the context is strictly bi-allelic
@@ -754,7 +743,7 @@ public class VariantContext implements Feature { // to enable tribble intergrati
      *
      * @return the set of alleles
      */
-    public Set<Allele> getAlleles() { return alleles; }
+    public List<Allele> getAlleles() { return alleles; }
 
     /**
      * Gets the alternate alleles.  This method should return all the alleles present at the location,
@@ -763,14 +752,8 @@ public class VariantContext implements Feature { // to enable tribble intergrati
      *
      * @return the set of alternate alleles
      */
-    public Set<Allele> getAlternateAlleles() {
+    public List<Allele> getAlternateAlleles() {
-        LinkedHashSet<Allele> altAlleles = new LinkedHashSet<Allele>();
+        return alleles.subList(1, alleles.size());
-        for ( Allele allele : alleles ) {
-            if ( allele.isNonReference() )
-                altAlleles.add(allele);
-        }
-
-        return Collections.unmodifiableSet(altAlleles);
     }
 
     /**
@@ -797,14 +780,7 @@ public class VariantContext implements Feature { // to enable tribble intergrati
      * @throws IllegalArgumentException if i is invalid
      */
     public Allele getAlternateAllele(int i) {
-        int n = 0;
+        return alleles.get(i+1);
-
-        for ( Allele allele : alleles ) {
-            if ( allele.isNonReference() && n++ == i )
-                return allele;
-        }
-
-        throw new IllegalArgumentException("Requested " + i + " alternative allele but there are only " + n + " alternative alleles " + this);
     }
 
     /**
@@ -813,8 +789,8 @@ public class VariantContext implements Feature { // to enable tribble intergrati
      *         regardless of ordering. Otherwise returns false.
      */
     public boolean hasSameAlternateAllelesAs ( VariantContext other ) {
-        Set<Allele> thisAlternateAlleles = getAlternateAlleles();
+        List<Allele> thisAlternateAlleles = getAlternateAlleles();
-        Set<Allele> otherAlternateAlleles = other.getAlternateAlleles();
+        List<Allele> otherAlternateAlleles = other.getAlternateAlleles();
 
         if ( thisAlternateAlleles.size() != otherAlternateAlleles.size() ) {
             return false;
@@ -1121,7 +1097,7 @@ public class VariantContext implements Feature { // to enable tribble intergrati
         if ( !hasGenotypes() )
             return;
 
-        Set<Allele> reportedAlleles = getAlleles();
+        List<Allele> reportedAlleles = getAlleles();
         Set<Allele> observedAlleles = new HashSet<Allele>();
         observedAlleles.add(getReference());
         for ( Genotype g : getGenotypes().values() ) {
@@ -1371,17 +1347,34 @@ public class VariantContext implements Feature { // to enable tribble intergrati
     }
 
     // protected basic manipulation routines
-    private static LinkedHashSet<Allele> alleleCollectionToSet(LinkedHashSet<Allele> dest, Collection<Allele> alleles) {
+    private static List<Allele> makeAlleles(Collection<Allele> alleles) {
-        for ( Allele a : alleles ) {
+        final List<Allele> alleleList = new ArrayList<Allele>(alleles.size());
-            for ( Allele b : dest ) {
+
+        boolean sawRef = false;
+        for ( final Allele a : alleles ) {
+            for ( final Allele b : alleleList ) {
                 if ( a.equals(b, true) )
                     throw new IllegalArgumentException("Duplicate allele added to VariantContext: " + a);
             }
 
-            dest.add(a);
+           // deal with the case where the first allele isn't the reference
+            if ( a.isReference() ) {
+                if ( sawRef )
+                    throw new IllegalArgumentException("Alleles for a VariantContext must contain a single reference allele: " + alleles);
+                alleleList.add(0, a);
+                sawRef = true;
+            }
+            else
+                alleleList.add(a);
         }
 
-        return dest;
+        if ( alleleList.isEmpty() )
+            throw new IllegalArgumentException("Cannot create a VariantContext with an empty allele list");
+
+        if ( alleleList.get(0).isNonReference() )
+            throw new IllegalArgumentException("Alleles for a VariantContext must contain a single reference allele: " + alleles);
+
+        return alleleList;
     }
 
     public static Map<String, Genotype> genotypeCollectionToMap(Map<String, Genotype> dest, Collection<Genotype> genotypes) {

public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextUtils.java

@@ -666,7 +666,7 @@ public class VariantContextUtils {
         return merged;
     }
 
-    private static final boolean hasPLIncompatibleAlleles(final Set<Allele> alleleSet1, final Set<Allele> alleleSet2) {
+    private static final boolean hasPLIncompatibleAlleles(final Collection<Allele> alleleSet1, final Collection<Allele> alleleSet2) {
         final Iterator<Allele> it1 = alleleSet1.iterator();
         final Iterator<Allele> it2 = alleleSet2.iterator();
 

public/java/test/org/broadinstitute/sting/gatk/walkers/diffengine/DiffableReaderUnitTest.java

@@ -70,7 +70,7 @@ public class DiffableReaderUnitTest extends BaseTest {
     private static void testLeaf(DiffNode rec, String field, Object expected) {
         DiffElement value = rec.getElement(field);
         Assert.assertNotNull(value, "Expected to see leaf named " + field + " in rec " + rec);
-        Assert.assertEquals(value.getValue().getValue(), expected, "Expected to leaf named " + field + " to have value " + expected + " in rec " + rec);
+        Assert.assertEquals(value.getValue().getValue(), expected, "Expected to see leaf named " + field + " to have value " + expected + " in rec " + rec + " but got instead " + value.getValue().getValue());
     }
 
     @Test(enabled = true, dependsOnMethods = "testPluggableDiffableReaders")
@@ -95,7 +95,7 @@ public class DiffableReaderUnitTest extends BaseTest {
         testLeaf(rec1, "POS", 2646);
         testLeaf(rec1, "ID", "rs62635284");
         testLeaf(rec1, "REF", Allele.create("G", true));
-        testLeaf(rec1, "ALT", new HashSet<Allele>(Arrays.asList(Allele.create("A"))));
+        testLeaf(rec1, "ALT", Arrays.asList(Allele.create("A")));
         testLeaf(rec1, "QUAL", 0.15);
         testLeaf(rec1, "FILTER", Collections.<Object>emptySet());
         testLeaf(rec1, "AC", "2");

public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUnitTest.java

@@ -13,6 +13,7 @@ import org.testng.Assert;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 
@@ -261,11 +262,16 @@ public class VariantContextUnitTest extends BaseTest {
         new VariantContext("test", insLoc, insLocStart, insLocStop, Arrays.asList(delRef, del));
     }
 
-    @Test (expectedExceptions = IllegalStateException.class)
+    @Test (expectedExceptions = IllegalArgumentException.class)
     public void testBadConstructorArgs3() {
         new VariantContext("test", insLoc, insLocStart, insLocStop, Arrays.asList(del));
     }
 
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testBadConstructorArgs4() {
+        new VariantContext("test", insLoc, insLocStart, insLocStop, Collections.<Allele>emptyList());
+    }
+
     @Test (expectedExceptions = IllegalArgumentException.class)
     public void testBadConstructorArgsDuplicateAlleles1() {
         new VariantContext("test", insLoc, insLocStart, insLocStop, Arrays.asList(Aref, T, T));

public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUtilsUnitTest.java

@@ -439,7 +439,7 @@ public class VariantContextUtilsUnitTest extends BaseTest {
         new MergeGenotypesTest("PerserveAlleles", "1,2",
                 makeVC("1", Arrays.asList(Aref, T), makeG("s1", Aref, T, 1)),
                 makeVC("2", Arrays.asList(Aref, C), makeG("s2", Aref, C, 2)),
-                makeVC("3", Arrays.asList(Aref, C, T), makeG("s1", Aref, T, 1), makeG("s2", Aref, C, 2)));
+                makeVC("3", Arrays.asList(Aref, T, C), makeG("s1", Aref, T, 1), makeG("s2", Aref, C, 2)));
 
         new MergeGenotypesTest("TakeGenotypePartialOverlap-1,2", "1,2",
                 makeVC("1", Arrays.asList(Aref, T), makeG("s1", Aref, T, 1)),