Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MATH-1597: LowDiscrepancySequence supplier/jump for Halton and Sobol #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

samyBadjoudj
Copy link
Contributor

No description provided.

public void testFirstSupplying() {
LowDiscrepancySequence sequence = new SobolSequenceGenerator(3);
Assert.assertArrayEquals(new double[]{0.0, 0.0, 0.0}, sequence.get(),1e-6);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this newline

@Test(expected = NotPositiveException.class)
public void testJumpNegativeIndex() {
LowDiscrepancySequence copyOfSeq = generator.jump(-5);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this newline

public void testFirstSupplying() {
LowDiscrepancySequence sequence = new HaltonSequenceGenerator(3);
Assert.assertArrayEquals(new double[]{0.0, 0.0, 0.0}, sequence.get(),1e-6);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this newline

@Test(expected = NotPositiveException.class)
public void testJumpNegativeIndex() {
LowDiscrepancySequence copyOfSeq = generator.jump(-5);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this newline

@samyBadjoudj samyBadjoudj force-pushed the feature/MATH-1597 branch 2 times, most recently from d85675f to 0412ad3 Compare July 6, 2021 05:59
@samyBadjoudj
Copy link
Contributor Author

done

@samyBadjoudj samyBadjoudj force-pushed the feature/MATH-1597 branch 2 times, most recently from 4ec560c to 187bd46 Compare July 6, 2021 07:18
@samyBadjoudj
Copy link
Contributor Author

check styles fixed

@coveralls
Copy link

coveralls commented Jul 6, 2021

Coverage Status

Coverage increased (+0.02%) to 90.46% when pulling ac65dca on samyBadjoudj:feature/MATH-1597 into dcf83c0 on apache:master.




@Test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remove these 2 extra lines as well

}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra line

* @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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can give the reference to the exception class as well org.apache.commons.math4.legacy.exception.NotPositiveException

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the J capital

Copy link
Contributor

@amarlearning amarlearning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants