de4b26cce325fdda34759775ce57233cf00aedce
java/com.sap.sse.common.test/src/com/sap/sse/common/test/KadaneExtremeSubarraysFinderTest.java
| ... | ... | @@ -62,8 +62,10 @@ public class KadaneExtremeSubarraysFinderTest { |
| 62 | 62 | assertEquals(0, finder.getStartIndexOfMaxSumSequence()); |
| 63 | 63 | assertEquals(4, finder.getEndIndexOfMaxSumSequence()); |
| 64 | 64 | finder.add(3, new ScalableDouble(-7)); |
| 65 | - assertEquals(9.0, finder.getMaxSum().divide(1.0), EPSILON); |
|
| 65 | + assertEquals(9.0, finder.getMaxSum().divide(1.0), EPSILON); // FIXME this test passes "coincidentally" because of the way getMaxSum() works while updating the aggregates... |
|
| 66 | 66 | assertEquals(4, finder.getStartIndexOfMaxSumSequence()); |
| 67 | 67 | assertEquals(5, finder.getEndIndexOfMaxSumSequence()); |
| 68 | 68 | } |
| 69 | + |
|
| 70 | + // TODO add tests using the remove(...) method |
|
| 69 | 71 | } |
java/com.sap.sse.common/src/com/sap/sse/common/scalablevalue/KadaneExtremeSubarraysFinder.java
| ... | ... | @@ -175,7 +175,7 @@ implements Serializable, Iterable<T> { |
| 175 | 175 | maxSumEndingAtPreviousIndex = newMaxSumEndingAt; |
| 176 | 176 | startIndexOfMaxSumSequenceIter.remove(); |
| 177 | 177 | startIndexOfMaxSumSequenceIter.add(nextGreaterOrEqualsThanSum ? i : startIndexOfMaxSumSequence.get(i-1)); |
| 178 | - if (compare(newMaxSumEndingAt, getMaxSum()) > 0) { |
|
| 178 | + if (compare(newMaxSumEndingAt, getMaxSum()) > 0) { // FIXME getMaxSum() cannot be asked while we are still updating; indices may have moved left (remove) or right (add)! |
|
| 179 | 179 | endIndexOfMaxSumSequence = i; |
| 180 | 180 | } |
| 181 | 181 | } else { |
| ... | ... | @@ -208,7 +208,19 @@ implements Serializable, Iterable<T> { |
| 208 | 208 | public synchronized void remove(int index) { |
| 209 | 209 | sequence.remove(index); |
| 210 | 210 | final ScalableValueWithDistance<ValueType, AveragesTo> maxSumEndingAtIndex = maxSumEndingAt.remove(index); |
| 211 | + startIndexOfMaxSumSequence.remove(index); |
|
| 212 | + if (endIndexOfMaxSumSequence > index) { |
|
| 213 | + endIndexOfMaxSumSequence--; |
|
| 214 | + } else if (endIndexOfMaxSumSequence == index) { |
|
| 215 | + // TODO but if endIndexOfMaxSumSequence == index, we have deleted the last element of the max sum sequence and need to re-evaluate |
|
| 216 | + } |
|
| 217 | + if (endIndexOfMinSumSequence > index) { |
|
| 218 | + endIndexOfMinSumSequence--; |
|
| 219 | + } else if (endIndexOfMinSumSequence == index) { |
|
| 220 | + // TODO but if endIndexOfMinSumSequence == index, we have deleted the last element of the min sum sequence and need to re-evaluate |
|
| 221 | + } |
|
| 211 | 222 | final ScalableValueWithDistance<ValueType, AveragesTo> minSumEndingAtIndex = minSumEndingAt.remove(index); |
| 223 | + startIndexOfMinSumSequence.remove(index); |
|
| 212 | 224 | update(index+1, maxSumEndingAtIndex, minSumEndingAtIndex); |
| 213 | 225 | } |
| 214 | 226 | |
| ... | ... | @@ -232,6 +244,11 @@ implements Serializable, Iterable<T> { |
| 232 | 244 | return startIndexOfMaxSumSequence.isEmpty() ? -1 : startIndexOfMaxSumSequence.get(endIndexOfMaxSumSequence); |
| 233 | 245 | } |
| 234 | 246 | |
| 247 | + /** |
|
| 248 | + * @return the index into {@link #sequence} holding the last element of the contiguous sub-sequence that has the |
|
| 249 | + * maximal sum; note that pointing <em>to</em> and not <em>after</em> the last element of that sequence is |
|
| 250 | + * slightly different from how indices may be handled in some other from/to collection operations. |
|
| 251 | + */ |
|
| 235 | 252 | public int getEndIndexOfMaxSumSequence() { |
| 236 | 253 | return endIndexOfMaxSumSequence; |
| 237 | 254 | } |
| ... | ... | @@ -240,6 +257,11 @@ implements Serializable, Iterable<T> { |
| 240 | 257 | return startIndexOfMinSumSequence.isEmpty() ? -1 : startIndexOfMinSumSequence.get(endIndexOfMinSumSequence); |
| 241 | 258 | } |
| 242 | 259 | |
| 260 | + /** |
|
| 261 | + * @return the index into {@link #sequence} holding the last element of the contiguous sub-sequence that has the |
|
| 262 | + * minimal sum; note that pointing <em>to</em> and not <em>after</em> the last element of that sequence is |
|
| 263 | + * slightly different from how indices may be handled in some other from/to collection operations. |
|
| 264 | + */ |
|
| 243 | 265 | public int getEndIndexOfMinSumSequence() { |
| 244 | 266 | return endIndexOfMinSumSequence; |
| 245 | 267 | } |