[
https://issues.apache.org/jira/browse/MATH-1597?focusedWorklogId=621296&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-621296
]
ASF GitHub Bot logged work on MATH-1597:
----------------------------------------
Author: ASF GitHub Bot
Created on: 10/Jul/21 15:38
Start Date: 10/Jul/21 15:38
Worklog Time Spent: 10m
Work Description: amarlearning commented on a change in pull request #190:
URL: https://github.com/apache/commons-math/pull/190#discussion_r667349534
##########
File path:
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/quasirandom/HaltonSequenceGenerator.java
##########
@@ -151,31 +159,44 @@ public HaltonSequenceGenerator(final int dimension, final
int[] bases, final int
* @param digit the j-th digit
* @return the scrambled digit
*/
- protected int scramble(final int i, final int j, final int b, final int
digit) {
+ protected long scramble(final int i, final int j, final int b, final long
digit) {
return weight != null ? (weight[i] * digit) % b : digit;
}
/**
- * Skip to the i-th point in the Halton sequence.
+ * jump to the i-th point in the Halton sequence.
* <p>
* This operation can be performed in O(1).
*
* @param index the index in the sequence to skip to
- * @return the i-th point in the Halton sequence
- * @throws org.apache.commons.math4.legacy.exception.NotPositiveException
NotPositiveException if index < 0
+ * @return the copy of this sequence
+ * @throws NotPositiveException if index < 0
Review comment:
We can give the reference to the exception class as well
`org.apache.commons.math4.legacy.exception.NotPositiveException`
##########
File path:
commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/quasirandom/HaltonSequenceGeneratorTest.java
##########
@@ -119,16 +118,30 @@ public void testConstructor2() throws Exception{
}
@Test
- public void testSkip() {
- double[] result = generator.skipTo(5);
+ public void testJump() {
+ LowDiscrepancySequence copyOfSeq = generator.jump(5);
+ double[] result = copyOfSeq.get();
Assert.assertArrayEquals(referenceValues[5], result, 1e-3);
- Assert.assertEquals(6, generator.getNextIndex());
+
for (int i = 6; i < referenceValues.length; i++) {
- result = generator.get();
+ result = copyOfSeq.get();
Assert.assertArrayEquals(referenceValues[i], result, 1e-3);
- Assert.assertEquals(i + 1, generator.getNextIndex());
}
}
+
+ @Test(expected = NotPositiveException.class)
+ public void testJumpNegativeIndex() {
+ LowDiscrepancySequence copyOfSeq = generator.jump(-5);
+ }
+
+
+
+ @Test
Review comment:
Can you please remove these 2 extra lines as well
##########
File path:
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/quasirandom/HaltonSequenceGenerator.java
##########
@@ -151,31 +159,44 @@ public HaltonSequenceGenerator(final int dimension, final
int[] bases, final int
* @param digit the j-th digit
* @return the scrambled digit
*/
- protected int scramble(final int i, final int j, final int b, final int
digit) {
+ protected long scramble(final int i, final int j, final int b, final long
digit) {
return weight != null ? (weight[i] * digit) % b : digit;
}
/**
- * Skip to the i-th point in the Halton sequence.
+ * jump to the i-th point in the Halton sequence.
Review comment:
Make the `J` capital
##########
File path:
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/quasirandom/HaltonSequenceGenerator.java
##########
@@ -151,31 +159,44 @@ public HaltonSequenceGenerator(final int dimension, final
int[] bases, final int
* @param digit the j-th digit
* @return the scrambled digit
*/
- protected int scramble(final int i, final int j, final int b, final int
digit) {
+ protected long scramble(final int i, final int j, final int b, final long
digit) {
return weight != null ? (weight[i] * digit) % b : digit;
}
/**
- * Skip to the i-th point in the Halton sequence.
+ * jump to the i-th point in the Halton sequence.
* <p>
* This operation can be performed in O(1).
*
* @param index the index in the sequence to skip to
- * @return the i-th point in the Halton sequence
- * @throws org.apache.commons.math4.legacy.exception.NotPositiveException
NotPositiveException if index < 0
+ * @return the copy of this sequence
+ * @throws NotPositiveException if index < 0
*/
- public double[] skipTo(final int index) {
+ @Override
+ public LowDiscrepancySequence jump(final long index) throws
NotPositiveException {
+ if(index < 0) {
+ throw new NotPositiveException(index);
+ }
+ HaltonSequenceGenerator copy = this.copy();
+ copy.performJump(index);
+ return copy;
+ }
+
+ /**
+ * Do jump at the index.
+ * @param index
+ */
+ private void performJump(long index) {
count = index;
- return get();
}
/**
- * Returns the index i of the next point in the Halton sequence that will
be returned
- * by calling {@link #get()}.
- *
- * @return the index of the next point
+ * Private constructor avoid side effects.
+ * @return copy of HaltonSequenceGenerator
*/
- public int getNextIndex() {
- return count;
+ private HaltonSequenceGenerator copy() {
+ return new HaltonSequenceGenerator(this);
}
+
+
Review comment:
extra space
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 621296)
Time Spent: 2h 20m (was: 2h 10m)
> Low-discrepancy sequence
> ------------------------
>
> Key: MATH-1597
> URL: https://issues.apache.org/jira/browse/MATH-1597
> Project: Commons Math
> Issue Type: Sub-task
> Reporter: Gilles Sadowski
> Priority: Minor
> Fix For: 4.0
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> Two low-discrepancy sequences are implemented in Commons Math (in package
> {{o.a.c.math4.legacy.random}}:
> * {{SobolSequenceGenerator}}
> * {{HaltonSequenceGenerator}}
> They both provide several methods:
> * {{nextVector()}}
> * {{skipTo(int)}}
> * {{getNextIndex()}}
> of which only the first is part of their common API through the
> {{RandomVectorGenerator}}.
> I propose to create an interface that would better represent the specific
> concept (and avoid the confusion with pseudo-random generators):
> {code}
> public interface LowDiscrepancySequence extends Supplier<double[]> { /* ...
> */}
> {code}
> Thus, instead of class {{SobolSequenceGenerator}} we'd have:
> {code}
> public class SobolSequence implements LowDiscrepancySequence {
> // ...
> }
> {code}
> This functionality could be moved to a new {{o.a.c.m.legacy.quasirandom}}
> package (?).
> Method {{skipTo}} could be replaced with an API similar to Commons RNG
> {{JumpableUniformRandomProvider}} (?).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
