You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gilles Sadowski <gi...@gmail.com> on 2021/11/30 16:50:19 UTC
[STATISTICS] Remove isSupportConnected from distribution interface
Hello.
Sorry for being late, but the change[1] in commit
d7c53ab3e05b09f2522f7b7ee78e96ad97defe95
impacts Commons Math: Method "isSupportConnected" is called in class
"AbtractRealDistribution".[2]
That class was ported to [STATISTICS] as "AbstractContinuousDistribution".
So now there are code duplications (beacuse ""AbtractRealDistribution" is
not public). Perhaps the "searchPlateau" functionality could be provided in
a public API (?).
Side-note: Why wasn't the issue caught by Jenkins?
Commons Math is not listed as being dependent on it.[3]
Regards,
Gilles
[1] https://issues.apache.org/jira/projects/STATISTICS/issues/STATISTICS-48
[2] https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/distribution/AbstractRealDistribution.java;h=3d79914bc6c19be3de255ce53763c1cae479ac33;hb=HEAD#l172
[3] https://ci-builds.apache.org/job/Commons/job/commons-statistics/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [STATISTICS] Remove isSupportConnected from distribution interface
Posted by Gilles Sadowski <gi...@gmail.com>.
>> [...]
> >
> > Note that isSupportConnected is not used by anything in Commons Math
> > either.
>
> OK, I missed that. So it's just a question of updating the code there.
FTR:
https://issues.apache.org/jira/browse/MATH-1636
Gilles
>> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [STATISTICS] Remove isSupportConnected from distribution interface
Posted by Alex Herbert <al...@gmail.com>.
On Tue, 30 Nov 2021 at 23:15, Gilles Sadowski <gi...@gmail.com> wrote:
> > > Side-note: Why wasn't the issue caught by Jenkins?
> > > Commons Math is not listed as being dependent on it.[3]
> >
> > So should we update Jenkins to have math build on changes to
> > Statistics, Numbers and RNG?
>
> Sure. I thought that it was inferred from the POM file...
It does not seem so. I added statistics, numbers, rng and geometry as
upstream projects to math:
https://ci-builds.apache.org/job/Commons/job/commons-math/
Alex
Re: [STATISTICS] Remove isSupportConnected from distribution interface
Posted by Gilles Sadowski <gi...@gmail.com>.
Le mar. 30 nov. 2021 à 19:41, Alex Herbert <al...@gmail.com> a écrit :
>
> On Tue, 30 Nov 2021 at 16:50, Gilles Sadowski <gi...@gmail.com> wrote:
> >
> > Hello.
> >
> > Sorry for being late, but the change[1] in commit
> > d7c53ab3e05b09f2522f7b7ee78e96ad97defe95
> > impacts Commons Math: Method "isSupportConnected" is called in class
> > "AbtractRealDistribution".[2]
> > That class was ported to [STATISTICS] as "AbstractContinuousDistribution".
> > So now there are code duplications (beacuse ""AbtractRealDistribution" is
> > not public). Perhaps the "searchPlateau" functionality could be provided in
> > a public API (?).
>
> I missed the fact that AbstractContinuousDistribution is not public.
> This makes the case for removing the isSupportConnected functionality
> stronger. I left it in so that it could be used by any extending
> classes but there are none in Commons Statistics. So the class
> supports functionality that is not used and cannot be inherited.
>
> Note that isSupportConnected is not used by anything in Commons Math
> either.
OK, I missed that. So it's just a question of updating the code there.
> It is also a rather poor implementation as it performs
> bisection between the current value and the lower bound. This may be
> very far from the current value and so the search for the connected
> support will be slow. I did not try and improve it as the function is
> not used.
>
> So here are some options:
>
> 1. The functionality in Commons Math can be deleted. A quick look and
> it appears the AbtractRealDistribution can also be made package
> private.
+1
> 2. AbstractContinuousDistribution in Statistics can be made public.
> 3. The functionality from AbstractContinuousDistribution can be
> transferred to Commons Math.
>
> I am leaning towards option 1. The method can live on in statistics in
> the event it is useful in the future.
>
> >
> > Side-note: Why wasn't the issue caught by Jenkins?
> > Commons Math is not listed as being dependent on it.[3]
>
> So should we update Jenkins to have math build on changes to
> Statistics, Numbers and RNG?
Sure. I thought that it was inferred from the POM file...
Thanks,
Gilles
>
> Alex
>
> >
> > Regards,
> > Gilles
> >
> > [1] https://issues.apache.org/jira/projects/STATISTICS/issues/STATISTICS-48
> > [2] https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/distribution/AbstractRealDistribution.java;h=3d79914bc6c19be3de255ce53763c1cae479ac33;hb=HEAD#l172
> > [3] https://ci-builds.apache.org/job/Commons/job/commons-statistics/
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [STATISTICS] Remove isSupportConnected from distribution interface
Posted by Alex Herbert <al...@gmail.com>.
On Tue, 30 Nov 2021 at 16:50, Gilles Sadowski <gi...@gmail.com> wrote:
>
> Hello.
>
> Sorry for being late, but the change[1] in commit
> d7c53ab3e05b09f2522f7b7ee78e96ad97defe95
> impacts Commons Math: Method "isSupportConnected" is called in class
> "AbtractRealDistribution".[2]
> That class was ported to [STATISTICS] as "AbstractContinuousDistribution".
> So now there are code duplications (beacuse ""AbtractRealDistribution" is
> not public). Perhaps the "searchPlateau" functionality could be provided in
> a public API (?).
I missed the fact that AbstractContinuousDistribution is not public.
This makes the case for removing the isSupportConnected functionality
stronger. I left it in so that it could be used by any extending
classes but there are none in Commons Statistics. So the class
supports functionality that is not used and cannot be inherited.
Note that isSupportConnected is not used by anything in Commons Math
either. It is also a rather poor implementation as it performs
bisection between the current value and the lower bound. This may be
very far from the current value and so the search for the connected
support will be slow. I did not try and improve it as the function is
not used.
So here are some options:
1. The functionality in Commons Math can be deleted. A quick look and
it appears the AbtractRealDistribution can also be made package
private.
2. AbstractContinuousDistribution in Statistics can be made public.
3. The functionality from AbstractContinuousDistribution can be
transferred to Commons Math.
I am leaning towards option 1. The method can live on in statistics in
the event it is useful in the future.
>
> Side-note: Why wasn't the issue caught by Jenkins?
> Commons Math is not listed as being dependent on it.[3]
So should we update Jenkins to have math build on changes to
Statistics, Numbers and RNG?
Alex
>
> Regards,
> Gilles
>
> [1] https://issues.apache.org/jira/projects/STATISTICS/issues/STATISTICS-48
> [2] https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/distribution/AbstractRealDistribution.java;h=3d79914bc6c19be3de255ce53763c1cae479ac33;hb=HEAD#l172
> [3] https://ci-builds.apache.org/job/Commons/job/commons-statistics/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org