commons-math icon indicating copy to clipboard operation
commons-math copied to clipboard

Feature/math 1563

Open avijitbasak opened this issue 2 years ago • 3 comments

Changes for task MATH-1618

avijitbasak avatar Sep 06 '21 13:09 avijitbasak

Thanks for the contribution. There are a few issues to address before a detailed review.

This will fail on checkstyle. You should replace all tab characters with 4 spaces and add new lines at the end of files. You can run a checkstyle report using:

mvn checkstyle:checkstyle

And then open target/site/checkstyle.html.

Before the CI build will pass you can check that the local build passes the default goal. This is:

mvn clean verify apache-rat:check checkstyle:check pmd:check spotbugs:check javadoc:javadoc

You can run the code check targets separately to get a report on what is wrong (outputs will be in the target/site directory):

mvn checkstyle:checkstyle
mvn pmd:pmd
mvn spotbugs:spotbugs

In general you should check that the module passes the default maven goal:

mvn

You have public interfaces with redundant public keywords on methods or properties.

There is a string property named CHROMOZOME which should be corrected.

The module directory name commons-math4-genetics should be commons-math-genetics. The 4 is used only in the artifactId.

The class RandomGenerator has a synchronized method to access a singleton. The effect of the synchronized keyword is meaningless here. The returned singleton is not thread-safe. A different design is required if the object is intended to be used across threads. A better approach is a single random generator per thread. The WELL_19937_C generator is not a very robust generator. There are faster and statistically better random generators available. For cross thread generic use you could try for example ThreadLocalRandomSource.current(RandomSource.XO_RO_SHI_RO_128_PP). If you want a thread-pool to avoid sequence collisions/overlap on the random output from the generator used by each thread then this will require creating threads with a child generator, each created from the same parent JumpableUniformRandomProvider.

aherbert avatar Sep 06 '21 14:09 aherbert

The following errors have been received after build. These are due to formatting of lambda expression. I have formatted the manually. It will be helpful if anyone can share eclipse configuration to resolve this.

[INFO] There are 6 errors reported by Checkstyle 8.29 with /home/travis/build/apache/commons-math/commons-math4-genetics/../src/main/resources/checkstyle/checkstyle.xml ruleset. [ERROR] src/test/java/org/apache/commons/math4/genetics/selection/TournamentSelectionTest.java:[51] (indentation) Indentation: 'block' child has incorrect indentation level 16, expected level should be 20. [ERROR] src/test/java/org/apache/commons/math4/genetics/selection/TournamentSelectionTest.java:[52] (indentation) Indentation: 'block rcurly' has incorrect indentation level 12, expected level should be 16. [ERROR] src/test/java/org/apache/commons/math4/genetics/dummy/DummyChromosome.java:[27] (indentation) Indentation: 'block' child has incorrect indentation level 12, expected level should be 16. [ERROR] src/test/java/org/apache/commons/math4/genetics/dummy/DummyChromosome.java:[28] (indentation) Indentation: 'block rcurly' has incorrect indentation level 8, expected level should be 12. [ERROR] src/test/java/org/apache/commons/math4/genetics/utils/ChromosomeRepresentationUtilsTest.java:[114] (indentation) Indentation: 'block' child has incorrect indentation level 20, expected level should be 12. [ERROR] src/test/java/org/apache/commons/math4/genetics/utils/ChromosomeRepresentationUtilsTest.java:[115] (indentation) Indentation: 'block rcurly' has incorrect indentation level 16, expected level should be 8. [INFO] ------------------------------------------------------------------------

avijitbasak avatar Sep 20 '21 13:09 avijitbasak

Coverage Status

Coverage decreased (-0.04%) to 90.418% when pulling 4b094b2d81c07f7cad203d2c3a54b71163633dd6 on avijitbasak:feature/MATH-1563 into 470bdbb5f35ddde4935ed32529386f36b656213e on apache:master.

coveralls avatar Sep 24 '21 08:09 coveralls