[GitHub] [commons-math] aherbert commented on pull request #197: Feature/math 1563

[GitBox <gi...@apache.org>]

aherbert commented on pull request #197:
URL: https://github.com/apache/commons-math/pull/197#issuecomment-913688042


   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`.
   


-- 
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-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org