You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/07/22 16:47:23 UTC

[GitHub] [commons-statistics] aherbert commented on a change in pull request #2: Simplify midpoint computation in AbstractDiscreteDistribution.solveInverseCumulativeProbability

aherbert commented on a change in pull request #2:
URL: https://github.com/apache/commons-statistics/pull/2#discussion_r458936797



##########
File path: commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/AbstractDiscreteDistribution.java
##########
@@ -120,25 +120,24 @@ public int inverseCumulativeProbability(final double p) {
     private int solveInverseCumulativeProbability(final double p,
                                                   int lower,
                                                   int upper) {
-        while (lower + 1 < upper) {
-            int xm = (lower + upper) / 2;
-            if (xm < lower || xm > upper) {
-                /*
-                 * Overflow.
-                 * There will never be an overflow in both calculation methods
-                 * for xm at the same time
-                 */
-                xm = lower + (upper - lower) / 2;

Review comment:
       Why don't we just use this formula all the time and avoid the if statement? This is single extra addition but does not require a branch.




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

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