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: <0ba285df6fd096adbf4be8529d1caaac@scarlet.be>
References: <CAGTHBS630cAe1+rQN4wxpoyax9AaXMhedWM4fYj-URUr+AzTSw@mail.gmail.com>
 <07a3cd49cfde1678a0623e7a4cf7da49@scarlet.be> <CAGTHBS449Oj2xmuY7SZXAbMpP-BjtP3T97hvjOF06bZTfUQ-2Q@mail.gmail.com>
 <0ba285df6fd096adbf4be8529d1caaac@scarlet.be>
From: Raymond DeCampo <ray@decampo.org>
Date: Sat, 16 Sep 2017 14:19:04 -0400
Message-ID: <CAGTHBS4MCdp9e9P=RK8nGunRL+cObwrAiG4t1wUEYp4oovWwNg@mail.gmail.com>
Subject: Re: [Math][Numbers] Move Field, etc. to numbers?
To: Commons Developers List <dev@commons.apache.org>
Content-Type: multipart/alternative; boundary="001a113d12926b0c7e0559528bcb"
archived-at: Sat, 16 Sep 2017 18:19:12 -0000

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

On Sat, Sep 16, 2017 at 1:34 PM, Gilles <gilles@harfang.homelinux.org>
wrote:

> On Sat, 16 Sep 2017 10:57:06 -0400, Raymond DeCampo wrote:
>
>> On Fri, Sep 15, 2017 at 9:59 PM, Gilles <gilles@harfang.homelinux.org>
>> wrote:
>>
>> On Fri, 15 Sep 2017 17:30:26 -0400, Raymond DeCampo wrote:
>>>
>>> So I was trying to work through MATH-1416, which is to remove code from
>>>> CM
>>>> which has been moved to CN and I ran into a snag when trying to replace
>>>> o.a.c.math4.f.BigFraction with the version from CN in
>>>> the KolmogorovSmirnovTest class.  This class uses o.a.c.m.f.BigFraction
>>>> as
>>>> an implementation of FieldElement which o.a.c.n.f.BigFraction is not
>>>> since
>>>> FieldElement is in CM and not in CN.
>>>>
>>>> I started down the road of creating an extension of
>>>> o.a.c.numbers.f.BigFraction which implemented FieldElement, but that
>>>> started to get sticky and complex and did not feel like the right
>>>> answer.
>>>>
>>>> I seems like the right answer is to move Field, FieldElement and
>>>> RealFieldElement into numbers and then BigFraction, Fraction, etc. can
>>>> implement FieldElement again.  This would mean no awkward bridge code to
>>>> stuff o.a.c.numbers.f.BigFraction into a FieldElement implementation.
>>>>
>>>> Re terminology a field is an abstract system of numbers so I don't see
>>>> an
>>>> issue there.
>>>>
>>>> Following the existing convention I would create a commons-numbers-field
>>>> submodule for the moved classes.
>>>>
>>>> Any objections?
>>>>
>>>>
>>> Yes, at least three:
>>>
>>> 1. "Field" and "FieldElement" themselves look awkward. I've alway
>>>    had a bad feeling about them, although I never had the urge to
>>>    find a way to fix them (or convince myself that there was no
>>>    better implementation).
>>>
>>>
>> This strikes me as the wrong time to try to refactor or replace them.
>>
>
> Once "Commons Numbers" is released, it will be too late; refactoring
> such a base class would entail a top-level package name change!
>
>
>
>> 2. The concept is fairly abstract (and indeed too "math-related"!).
>>>
>>>
>> Putting aside the idea that something is too math-related for CM or CN,
>> while the concept of a field is an abstract mathematical concept, it
>> strikes me that the interfaces based on it are very useful from a CS point
>> of view as well.
>>
>> The interfaces establish that a particular Number representation follows
>> certain rules and implements certain operations.  So if one develops an
>> algorithm which restricts itself to these rules it may be applied to many
>> different Number classes.
>>
>>
> Of course, I agree with the above.
> However there is a need to take various POV into account.
> One of them is usefulness of an abstract concept in a "basic"
> utility library like "Commons Numbers".  "Numbers" utilities
> are extracted from CM for a reason: because they are useful
> on their own.
> IIRC, there was a discussion whether we should go further than
> "Field" in taking the mathematician's POV. The consensus was
> "no" because there was no need in CM (and because CM was not
> intended to be an implementation of any and all math concepts!).
>
> So from the POV of a "Commons Numbers" developer, what is the
> added value of "Field"?
> [IMO none at the moment (but that could change).]


I'm not looking at it from the POV of a CN developer.  I am looking at it
from the POV of a CN user.  I believe this is covered in the following
paragraph you quoted.


>
>
> Since Java is strongly typed and does not support duck typing without
>> resorting to reflection gymnastics, there is currently no way to write an
>> algorithm using e.g. the add() method which could be applied to
>> o.a.c.n.f.Fraction and o.a.c.n.c.Complex without duplication, reflection
>> or
>> pointless wrapper extensions.
>>
>
> All I say is that I'm not convinced that "Field" and
> "FieldElement" as they are now, is the best way to allow
> that in either "Commons Numbers" or CM.
>
> Have we explored other possible APIs?
> E.g.:
> ----
> interface Add<T> {
>   T add(T other);
> }
> ----
> etc

.
>
> Can we leverage something from Java 8?
>
> My feeling is that "Field" and "FieldElement" don't
> look right...  But I could be wrong.  That they are
> in CM is not a convincing argument.
>
> 3. There is a single identified need, in "KolmogorovSmirnovTest";
>>>    and it is internal to it.
>>>
>>> Thus, I'd be wary to pollute "Numbers" with what seems an ad-hoc
>>> API that has a single CM class as the only target.
>>>
>>>
>> From what I have seen Field and FieldElement are used extensively in CM
>> and
>> hardly seem like an ad-hoc API.  I suspect this will be the tip of the
>> proverbial rabbit hole to mix a few metaphors.
>>
>
> I meant "ad-hoc" as the thing needed to make "BigFraction" work
> in "KolmogorovSmirnovTest". And at the same time, I question
> the design of that "KolmogorovSmirnovTest" class and the
> usefulness of the "Field"-related computations (as part of a
> library mainly based on the "double" data type).
>
> Recall that we are in that position because many codes are
> not supported.
> My rationale is that since we don't understand the details
> of why "Filed" is used in "KolmogorovSmirnovTest", we should
> not impose that API on "Commons Numbers".
>
> Since "Field" is used by CM, it should stay there until we
> can see the added value. [At that time we can add interfaces
> to the API, hopefully without breaking compatibility.]
>
>
>>> I'd much prefer that we create a "FieldElement" implementation
>>> that is a bridge to "BigFraction" (from "Numbers") and make the
>>> necessary changes in "KolmogorovSmirnovTest".
>>> I've done it (mostly: class renames are in order that can be done
>>> once "BigFraction" is deleted from CM); if you file report in JIRA,
>>> I'll attach the files to it, for you to review.
>>> [Admittingly, code in "KolmogorovSmirnovTest" will become slightly
>>> more convoluted because of the bridge; however IMHO the whole class
>>> could welcome a thorough cleanup...]
>>> Until issue (1) above is solved, we should even make the bridge
>>> utilities internal to "KolmogorovSmirnovTest".
>>>
>>>
>>> Unless I have misunderstood, issue (1) appears to be that you do not like
>> the Field and FieldElement interfaces.  I haven't a clue how to go about
>> resolving that.
>>
>
> I cringe on
> * the name (Are we sure that any class implementing the API would
>   define a field?)
> * the use of reflection
> * the use of wildcard
> * the use of "on-demand" loading
>
> Again, perhaps there is no way around.
> But no documentation justifies those choices.
>
>
>> I don't have time to go spelunking into the KolmogorovSmirnovTest now, I
>> was looking for something I could knock off without a big time commitment
>> to help get CN ready for release.  So I will skip the removal of
>> BigFraction for now.
>>
>
> I don't understand this line of reasoning.
> I made the changes already, and "KolmogorovSmirnovTest" still
> passes all its unit tests.

  Is there another place in CM
> where the "Field" nature of "BigFraction" is needed?
> If not, it can be replaced by its "Numbers" equivalent.
> I.e. you could
>  1. do most of what you intended,
>  2. open a JIRA issue about "KolmogorovSmirnovTest"
> which I'll try to fix as I proposed.
>
> WDYT?
>
>
To be blunt I don't agree with the approach so I am not about to devote
time and effort to supporting it.  I'm also not particularly interested in
arguing about it.  So I have completed the portions where we have agreement
and left the rest.

Summarizing the other issues I found while trying to eliminate BigFraction,
Fraction and Complex from CM:

I found nine test classes which use FractionField, those would need to be
rewritten.  I imagine they could use a different Field implementation
without a great deal of difficulty.

I also found a couple of public methods in MatrixUtils that I imagine will
need to be deleted.  I don't think they would be missed but OTOH they were
added because somebody wanted them.

What you do propose to do about the ComplexField, FractionField and
BigFractionField classes?  These were part of the CM API in the last
release are is the plan to just drop them?

--001a113d12926b0c7e0559528bcb--