From 04cb6a433a7ae367e57454c069222ccb830a03f1 Mon Sep 17 00:00:00 2001 From: kiramt Date: Fri, 29 Sep 2017 15:01:08 +0100 Subject: [PATCH] JAL-2674 Tidies --- .../org/jalview/HiddenColsIteratorsBenchmark.java | 16 -- src/jalview/datamodel/HiddenColumns.java | 200 +++++++------------- test/jalview/datamodel/HiddenColumnsTest.java | 44 ----- 3 files changed, 65 insertions(+), 195 deletions(-) diff --git a/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java b/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java index e97baba..1710c85 100644 --- a/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java +++ b/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java @@ -137,20 +137,4 @@ public class HiddenColsIteratorsBenchmark { } return blockStart; } - - @Benchmark - @BenchmarkMode({Mode.Throughput}) - public int benchLoop1(HiddenColsAndStartState tstate) - { - int res = 0; - int startx = tstate.visibleColumn; - List positions = tstate.h.findHiddenRegionPositions(startx, startx+60); - - for (int pos : positions) - { - res = pos - startx; - Blackhole.consumeCPU(5); - } - return res; - } } diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index 5554be0..aa34461 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -24,7 +24,6 @@ import jalview.util.Comparison; import java.util.ArrayList; import java.util.BitSet; -import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; @@ -117,19 +116,6 @@ public class HiddenColumns } /** - * This method is used to return all the HiddenColumn regions and is intended - * to remain private. External callers which need a copy of the regions can - * call getHiddenColumnsCopyAsList. - * - * @return empty list or List of hidden column intervals - */ - private List getHiddenRegions() - { - return hiddenColumns == null ? Collections. emptyList() - : hiddenColumns; - } - - /** * Output regions data as a string. String is in the format: * reg0[0]reg0[1]reg1[0]reg1[1] ... regn[1] * @@ -405,61 +391,7 @@ public class HiddenColumns } - /** - * Use this method to determine the set of hiddenRegion start positions - * between absolute position and absolute position - * - * @param start - * absolute residue to start from - * @param end - * absolute residue to end at - * - * @return list of column numbers in *visible* view where hidden regions start - */ - public List findHiddenRegionPositions(int start, int end) - { - try - { - LOCK.readLock().lock(); - List positions = null; - - if (hiddenColumns != null) - { - positions = new ArrayList<>(hiddenColumns.size()); - // navigate to start, keeping count of hidden columns - int i = 0; - int hiddenSoFar = 0; - while ((i < hiddenColumns.size()) - && (hiddenColumns.get(i)[0] < start)) - { - int[] region = hiddenColumns.get(i); - hiddenSoFar += region[1] - region[0] + 1; - i++; - } - - // iterate from start to end, adding start positions of each - // hidden region. Positions are visible columns count, not absolute - while (i < hiddenColumns.size() - && (hiddenColumns.get(i)[0] < end)) - { - int[] region = hiddenColumns.get(i); - positions.add(region[0] - hiddenSoFar); - hiddenSoFar += region[1] - region[0] + 1; - i++; - } - } - else - { - positions = new ArrayList<>(); - } - - return positions; - } finally - { - LOCK.readLock().unlock(); - } - } /** * This method returns the rightmost limit of a region of an alignment with @@ -796,9 +728,8 @@ public class HiddenColumns // Simply walk along the sequence whilst watching for hidden column // boundaries - List regions = getHiddenRegions(); + Iterator regions = iterator(); int spos = fpos; - int rcount = 0; int hideStart = seq.getLength(); int hideEnd = -1; int visPrev = 0; @@ -819,9 +750,9 @@ public class HiddenColumns } lastP = p; // update hidden region start/end - while (hideEnd < p && rcount < regions.size()) + while (hideEnd < p && regions.hasNext()) { - int[] region = regions.get(rcount++); + int[] region = regions.next(); visPrev = visNext; visNext += region[0] - visPrev; hideStart = region[0]; @@ -866,7 +797,8 @@ public class HiddenColumns */ public void makeVisibleAnnotation(AlignmentAnnotation alignmentAnnotation) { - makeVisibleAnnotation(-1, -1, alignmentAnnotation); + makeVisibleAnnotation(0, alignmentAnnotation.annotations.length, + alignmentAnnotation); } /** @@ -892,11 +824,6 @@ public class HiddenColumns if (alignmentAnnotation.annotations != null) { - if (start == end && end == -1) - { - startFrom = 0; - endAt = alignmentAnnotation.annotations.length; - } if (hiddenColumns != null && hiddenColumns.size() > 0) { removeHiddenAnnotation(startFrom, endAt, alignmentAnnotation); @@ -1129,60 +1056,69 @@ public class HiddenColumns private void propagateInsertions(SequenceI profileseq, AlignmentI al, SequenceI origseq) { - char gc = al.getGapCharacter(); - - // take the set of hidden columns, and the set of gaps in origseq, - // and remove all the hidden gaps from hiddenColumns - - // first get the gaps as a Bitset - BitSet gaps = origseq.gapBitset(); - - // now calculate hidden ^ not(gap) - BitSet hidden = new BitSet(); - markHiddenRegions(hidden); - hidden.andNot(gaps); - hiddenColumns = null; - this.hideMarkedBits(hidden); - - // for each sequence in the alignment, except the profile sequence, - // insert gaps corresponding to each hidden region - // but where each hidden column region is shifted backwards by the number of - // preceding visible gaps - // update hidden columns at the same time - Iterator regions = iterator(); - ArrayList newhidden = new ArrayList<>(); - - int numGapsBefore = 0; - int gapPosition = 0; - while (regions.hasNext()) + try { - // get region coordinates accounting for gaps - // we can rely on gaps not being *in* hidden regions because we already - // removed those - int[] region = regions.next(); - while (gapPosition < region[0]) - { - gapPosition++; - if (gaps.get(gapPosition)) + LOCK.writeLock().lock(); + + char gc = al.getGapCharacter(); + + // take the set of hidden columns, and the set of gaps in origseq, + // and remove all the hidden gaps from hiddenColumns + + // first get the gaps as a Bitset + BitSet gaps = origseq.gapBitset(); + + // now calculate hidden ^ not(gap) + BitSet hidden = new BitSet(); + markHiddenRegions(hidden); + hidden.andNot(gaps); + hiddenColumns = null; + this.hideMarkedBits(hidden); + + // for each sequence in the alignment, except the profile sequence, + // insert gaps corresponding to each hidden region + // but where each hidden column region is shifted backwards by the number + // of + // preceding visible gaps + // update hidden columns at the same time + Iterator regions = iterator(); + ArrayList newhidden = new ArrayList<>(); + + int numGapsBefore = 0; + int gapPosition = 0; + while (regions.hasNext()) + { + // get region coordinates accounting for gaps + // we can rely on gaps not being *in* hidden regions because we already + // removed those + int[] region = regions.next(); + while (gapPosition < region[0]) { - numGapsBefore++; + gapPosition++; + if (gaps.get(gapPosition)) + { + numGapsBefore++; + } } - } - int left = region[0] - numGapsBefore; - int right = region[1] - numGapsBefore; - newhidden.add(new int[] { left, right }); + int left = region[0] - numGapsBefore; + int right = region[1] - numGapsBefore; + newhidden.add(new int[] { left, right }); - // make a string with number of gaps = length of hidden region - StringBuffer sb = new StringBuffer(); - for (int s = 0; s < right - left + 1; s++) - { - sb.append(gc); - } - padGaps(sb, left, profileseq, al); + // make a string with number of gaps = length of hidden region + StringBuffer sb = new StringBuffer(); + for (int s = 0; s < right - left + 1; s++) + { + sb.append(gc); + } + padGaps(sb, left, profileseq, al); + } + hiddenColumns = newhidden; + } finally + { + LOCK.writeLock().unlock(); } - hiddenColumns = newhidden; } /** @@ -1712,7 +1648,8 @@ public class HiddenColumns if (hiddenColumns != null) { int i = 0; - for (i = 0; i < hiddenColumns.size(); ++i) + for (i = 0; i < hiddenColumns.size() + && (current < hiddenColumns.get(i)[0]); ++i) { if (current >= hiddenColumns.get(i)[0] && current <= hiddenColumns.get(i)[1]) @@ -1721,15 +1658,12 @@ public class HiddenColumns current = hiddenColumns.get(i)[1] + 1; next = current; } - if (current < hiddenColumns.get(i)[0]) - { - break; - } } lasthiddenregion = i - 1; - for (i = hiddenColumns.size() - 1; i >= 0; --i) + for (i = hiddenColumns.size() - 1; i >= 0 + && (last > hiddenColumns.get(i)[1]); --i) { if (last >= hiddenColumns.get(i)[0] && last <= hiddenColumns.get(i)[1]) @@ -1737,10 +1671,6 @@ public class HiddenColumns // last is hidden, move to left last = hiddenColumns.get(i)[0] - 1; } - if (last > hiddenColumns.get(i)[1]) - { - break; - } } // make a local copy of the bit we need diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index a3d4623..192a822 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -32,7 +32,6 @@ import jalview.util.Comparison; import java.util.Arrays; import java.util.BitSet; import java.util.Iterator; -import java.util.List; import java.util.Random; import org.testng.annotations.BeforeClass; @@ -843,49 +842,6 @@ public class HiddenColumnsTest } @Test(groups = { "Functional" }) - public void testFindHiddenRegionPositions() - { - HiddenColumns hc = new HiddenColumns(); - - List positions = hc.findHiddenRegionPositions(0, 20); - assertTrue(positions.isEmpty()); - - hc.hideColumns(3, 7); - hc.hideColumns(10, 10); - hc.hideColumns(14, 15); - - positions = hc.findHiddenRegionPositions(0, 20); - assertEquals(3, positions.size()); - assertEquals(3, positions.get(0).intValue()); - assertEquals(5, positions.get(1).intValue()); - assertEquals(8, positions.get(2).intValue()); - - positions = hc.findHiddenRegionPositions(7, 20); - assertEquals(2, positions.size()); - assertEquals(5, positions.get(0).intValue()); - assertEquals(8, positions.get(1).intValue()); - - positions = hc.findHiddenRegionPositions(11, 13); - assertEquals(0, positions.size()); - - positions = hc.findHiddenRegionPositions(7, 20); - assertEquals(2, positions.size()); - assertEquals(5, positions.get(0).intValue()); - assertEquals(8, positions.get(1).intValue()); - - positions = hc.findHiddenRegionPositions(0, 1); - assertEquals(0, positions.size()); - - positions = hc.findHiddenRegionPositions(17, 20); - assertEquals(0, positions.size()); - - positions = hc.findHiddenRegionPositions(10, 15); - assertEquals(2, positions.size()); - assertEquals(5, positions.get(0).intValue()); - assertEquals(8, positions.get(1).intValue()); - } - - @Test(groups = { "Functional" }) public void testRegionsToString() { HiddenColumns hc = new HiddenColumns(); -- 1.7.10.2