Hello.

Le mar. 7 sept. 2021 à 14:11, Erik Svensson <erik.svens...@nasdaq.com> a écrit :
>
> Hi!
>
> On 2021-09-06, 21:21, "Gilles Sadowski" <gillese...@gmail.com> wrote:
>
>     WARNING - External email; exercise caution.
>
>     Hi.
>
>     Le lun. 6 sept. 2021 à 15:26, Erik Svensson <erik.svens...@nasdaq.com> a 
> écrit :
>     >
>     > On 2021-09-02, 18:11, "Gilles Sadowski" <gillese...@gmail.com> wrote:
>     >
>     >     WARNING - External email; exercise caution.
>     >
>     >     Le jeu. 2 sept. 2021 à 16:30, Erik Svensson 
> <erik.svens...@nasdaq.com> a écrit :
>     >     >
>     >     > Hi!
>     >     >
>     >     >
>     >     >
>     >     > I’ve implemented profiles, where you can choose to use either 
> AccurateMath or jlM and it works on j8.
>     >
>     >     Can you provide a link to the code?
>     >
>     > https://github.com/perf-coder/commons-math/tree/math-profiles-j8-am
>
>    [...]
>
>     There is a feeling of things being upside down: You've moved all the
>     "AccurateMath" code to a new "AccurateMathImpl" class (whereas the
>     *implementation* was indeed the former).
>
> My reasoning was that that by coring out AM I wouldn't have to rewrite all 
> the downstream code that uses AM and that could benefit from a better 
> performing implementation.

I guessed that it was the reason, but avoiding a one-time search/replace
does not warrant the confusing naming.
In my proposal, the name "JdkMath" makes an explicit connection to
the JDK's class and a one-liner Javadoc is sufficient for the user to
know what this is about.

>
>     Then, the new "AccurateMath" now computes results that depend on
>     the selected "profile", entailing that there is no accuracy guarantee!
>
> That is true, see my above comment. One solution would be to have a wrapper 
> class (as my AM is) with another name that defaults to AM for implementation.

I'm missing the need for all the classes (except one for doing the mapping).
Could you please start a new thread for listing the advantages/drawbacks
of various alternative changes together with their aimed goal(s)/non-goal(s)?

>     The "MathAPI" class conjures the idea of some "new" API (defined by
>     this project) whereas the API is defined by the contents of the JDK's
>     "Math" class.
>
> Finding good names is hard.

You are right.  But in this case the first issue to resolve is to determine
how many classes we actually need in order to achieve the goals.

>
>     Allowing any application to globally change the implementation (through
>     "MathProfile") seems quite unsafe.
>
> That was just an half-baked idea I had that you might want to insert your own 
> profile.

I suspect that this will be a can of worms.
For someone to experiment with one's own implementation, it's safer to
fork/clone the source code and create a modified version of (e.g.) the
"JdkMath" class with an additional value for the enum.  [Then, if there is
value to the new profile, it should be contributed back here...]

>
>     What about the following approach:
>     ---CUT---
>     public final class JdkMath {
>         /** Property identifier. */
>         private static final String PROPERTY_KEY =
>     "org.apache.commons.math.jdkmath";
>         /** max(x, y). */
>         private static final DoubleBinaryOperator max;
>         /** sin(x). */
>         private static final DoubleUnaryOperator sin;
>         /** random(). */
>         private static final DoubleSupplier random;
>
>         /** Available implementations of {@link Math} functions. */
>         public enum Impl {
>             /** {@link AccurateMath Commons Math}. */
>             CM,
>             /** {@link Math JDK}. */
>             JDK
>         }
>
>         static {
>             final String prop = System.getProperty(PROPERTY_KEY);
>             final Impl impl = prop != null ?
>                 Impl.valueOf(prop) :
>                 Impl.CM;
>
>             switch(impl) {
>             case CM:
>                 max = AccurateMath::max;
>                 sin = AccurateMath::sin;
>                 random = AccurateMath::random;
>                 break;
>
>             case JDK:
>                 max = Math::max;
>                 sin = Math::sin;
>                 random = Math::random;
>                 break;
>
>             default:
>                 throw new IllegalStateException("Internal error"); //
>     Should never happen.
>             }
>         }
>
>         /**
>          * @param x Number.
>          * @param y Number.
>          * @return max(x, y).
>          */
>         public static double max(double x,
>                                  double y) {
>             return max.applyAsDouble(x, y);
>         }
>
>         /**
>          * @param x Number.
>          * @return sin(x).
>          */
>         public static double sin(double x) {
>             return sin.applyAsDouble(x);
>         }
>
>         /**
>          * @return a random number between 0 and 1.
>          */
>         public static double random() {
>             return random.getAsDouble();
>         }
>
>         // ...
>     }
>     ---CUT---
>     ?
>
> Note that this will not solve the problem of failing the StrictMath 
> conformance test.

Please state/explain this in the new thread.

> I'll check it out with JMH. The performance improvements from 
> @IntrinsicCandidate are impressive but quite fragile. It does not need much 
> to totally consume it.

Ditto with this expectation.
And: Please share your JMH code (even better: a PR to mimic, within CM,
the JMH module in "Commons RNG"[1]).

> I have a branch that uses enum:s for math profiles but I thought it might be 
> interesting to be able to insert your own profile.

Cf. above.

Thanks,
Gilles

[1] 
https://gitbox.apache.org/repos/asf?p=commons-rng.git;a=tree;f=commons-rng-examples/examples-jmh;h=bd58f92b338d71a147d22f33351b96e4fe35c56d;hb=HEAD

>>>  [...]

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

Reply via email to