Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
MIME-Version: 1.0
In-Reply-To: <238bcf7a2899ea47bead71bbf22b2aee@scarlet.be>
References: <238bcf7a2899ea47bead71bbf22b2aee@scarlet.be>
From: Stian Soiland-Reyes <stain@apache.org>
Date: Mon, 8 May 2017 10:01:48 +0100
Message-ID: <CAMBJEmWVZygoGS35M0TdFiPkPY7Jih+kCFm_dUsvC83yvKpXaA@mail.gmail.com>
Subject: Re: [Numbers] NUMBERS-33
To: Commons Developers List <dev@commons.apache.org>
Content-Type: multipart/alternative; boundary=001a113542e041ab81054eff7d3d
archived-at: Mon, 08 May 2017 09:01:57 -0000

--001a113542e041ab81054eff7d3d
Content-Type: text/plain; charset=UTF-8

Thanks!

Not being a mathematician I am afraid I have not been able to review the
correctness of the equation use.

Overall the addition looks good and well commented. The javadoc is a bit
sparse and could do with some more hyperlinks.

 I have some code design questions you may be able to clarify:

* Why have static methods all called value()? Static methods are good as
they can be imported independently (and without wasting object state), yet
by calling all the public methods .value() only one can be imported, and
with a meaningless name; we are forced instead to always call them via the
class Gamma.value()

* Why do some of the package private classes (e.g. InvGamma1pm1) have a
needless static singleton instance and a single method (and no state), when
here as well a static method would do? The only reason I can see for this
is to implement interfaces or subclasses, yet none of those seem to be
present, and these could rather follow the same pattern as the public API
with private constructors.

Won't this loop loose precision on digamma? (Could be called up to 49 times)

while (x < C_LIMIT) {
+ digamma -= 1 / x;
+ x += 1;
+ }

In this loop I was confused as the formula does not at first seem to match
the code:
final double inv = 1 / (x * x);
+ // 1 1 1 1
+ // log(x) - --- - ------ + ------- - -------
+ // 2 x 12 x^2 120 x^4 252 x^6
+ digamma += Math.log(x) - 0.5 / x - inv * (F_1_12 + inv * (F_1_120 -
F_1_252 * inv));
+
+ return digamma;

Here and other places:
/** Helper. */
+ private static final InvGamma1pm1 INV_GAMMA_1P_M1 = InvGamma1pm1.instance;

Unless there were state to initialize in certain order I don't see the
benefit of this field wastage over just calling InvGamma1pm1.value()
directly.

Based on the <em>NSWC Library of Mathematics Subroutines</em> double
+ * precision implementation, {@code DGAMMA}.

What is the license/url of this nswc code? What is the nature of "based on"
here?


On 8 May 2017 1:30 am, "Gilles" <gilles@harfang.homelinux.org> wrote:

Hi.

Please review branch "task_NUMBERS-33__Gamma":
  https://issues.apache.org/jira/browse/NUMBERS-33

Thanks,
Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org

--001a113542e041ab81054eff7d3d--