from:"Sébastien Brisard"

Hi Thomas,

> Hi,
>
> I have investigated MATH-627, and I think we can remove the additional null
> checks (tested locally):
>
>  * the iterators in RealVector and ArrayRealVector never return null as a
> result from next()
>  * next() is only called when hasNext() returns true
>
> The thing I want to discuss is that RealVector is only an abstract class,
> meaning that it may be used as base class for user specific implementations
> (like in the test cases).
> Now any implementation has to provide an iterator that follows the same
> assumptions as in the base implementation, i.e. never return a null entry.
>
> Imo, an implementation that returns a null entry although the iterator
> states it has more entries is buggy and thus should fail (atm, returning a
> null entry would terminate while loops iterating over such an iterator).
> So I think it should be safe to remove the null checks and add a note to
> the javadoc that any derived classes have to return an iterator following
> the same convention.
>
> Any objections?
>
No objections on my side!
Sébastien


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

[math] Deprecating

Hi,
in this thread,
http://mail-archives.apache.org/mod_mbox/commons-dev/201204.mbox/%3C20120418122114.GB32074%40dusk.harfang.homelinux.org%3E
I suggested that we deprecate guessParametersErrors() and replace it
with getParametersStandardDeviation(), since the implemented formula
seems to be the accepted estimate for the sd of the optimized
parameters.
There was no reaction, but I don't think that meant agreement (!).
Although I do think that guessParametersErrors() is a bit vague (if
not confusing), I'm going to drop this proposal unless someone says he
supports it. As far as I am concerned, MATH-784 would then be resolved
(I stiil need to inquire about getSigma()).

Best regards,
Sébastien


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

Re: [math] Deprecating

Hi,
2012/5/3 Gilles Sadowski :
> Hello.
>
>> in this thread,
>> http://mail-archives.apache.org/mod_mbox/commons-dev/201204.mbox/%3C20120418122114.GB32074%40dusk.harfang.homelinux.org%3E
>> I suggested that we deprecate guessParametersErrors() and replace it
>> with getParametersStandardDeviation(), since the implemented formula
>> seems to be the accepted estimate for the sd of the optimized
>> parameters.
>> There was no reaction, but I don't think that meant agreement (!).
>
> There isn't even an agreement that this method should not be deleted... :-)
> Indeed, people around here (working in astronomy) maintain that the error
> they'd publish is the square-root of the diagonal element of the covariance
> matrix.
>
> I've been busy coding a trivial example aimed at comparing the two formulae.
> I'll probably attach the file to issue MATH-784 tomorrow.
>
>> Although I do think that guessParametersErrors() is a bit vague (if
>> not confusing), I'm going to drop this proposal unless someone says he
>> supports it. As far as I am concerned, MATH-784 would then be resolved
>> (I stiil need to inquire about getSigma()).
>
> As hinted above, what we use here is
>
>  for (int i = 0; i < covMat.length; i++) {
>    sigma[i] = FastMath.sqrt(covMat[i][i]);
>  }
>
> Thus, a
>
>  double[] getSigma() { /* ... */ }
>
> method would be nice to have, IMO.
> However, we should make quite clear in the Javadoc what is the meaning of
> the returned "error" (in particular, as most users are no more statistician
> than we are, there should probably be a simple explanation of when to use
> which).
>
>
> Best,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
this is interesting stuff. I can make the addition of the required
getSigma() method. Only, I'm reluctant to call it getSigma(), as sigma
would suggest a standard deviation, which in fact is returned by
guessParametersErrors().

The only reference I was able to find where the diagonal elements are
directly used is numerical recipes, and if I remember correctly, I
think it was called $\Delta a$, and was derived from the $\chi-2$
distribution. So it basically returns a confidence interval, I think,
while guessParametersErrors truly returns a standard deviation. Is
that correct? In that case, we could call getSigma() just that:
getConfidenceInterval (the associated probability might even be
specified as a parameter, or decided once and for all in the javadoc).
Is

Gilles, the best thing to do would probably be a Monte-Carlo
simulation, with several realizations of the same dataset. Then, you
would have a distribution of the parameters, from which you would be
able to compute the standard deviation. If you want, I can do that.

Meanwhile, I've asked for the Bevington book, but inter-library loans
can tae a while...

Best regards,
Sébastien


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

Re: [math] Deprecating

BTW, I've just noticed the very poor subject of this thread. I'm sorry
for that, was too quick clicking the "send" button.
Sébastien


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

Re: [math] Deprecating

Hi,
attached to MATH-784 (https://issues.apache.org/jira/browse/MATH-784)
is a Monte-Carlo simulation which might be used to explore this issue.
For the time being, it confirms that
* guessParametersErrors() indeed estimates the sd on the parameters,
* the sqrt of the diagonal coefficients of the covariance matrix also
provide a good estimate of these standard deviations, and also the 68%
confidence interval (as announced in Numerical Recipes, 15.6).

Although these values are very close, I do think they do not really
have the same mathematical meaning.

What remains to be explored
* use observations which are not normally distributed (e.g. Poisson?),
* use smaller sets of observations, which should emphasize the
difference between guessParametersErrors() and the sqrt of the
diagonal coefficients.

Will do that later.
Sébastien


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

Re: [math] Deprecating

Hi Dimitri,
I think we agree on the two different concepts (sd of the parameters,
and confidence interval), and we also agree that the terminology in CM
should be precise.
For the moment, the formula for guessParametersErrors is
sqrt(chi^2 / dof * cov[i][i])
this is I believe the formula for the standard deviation on the parameters.

As far as I understood sqrt(cov[i][i]) is the half-width of the 68%
confidence interval.

I hope I got it correctly!

Now, do you call sigma the first or the second expression? I would
think the first expression should be called sigma, since it is truly a
standard deviation. But you are more used than I am to this kind of
statistical analyses.

I really want to carry out a new monte-carlo simulation with a small
number of dofs, so that the difference between the two fomulae will be
greater than observed at the moment.

Sébastien


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

Re: [math] Deprecating

Hi Dimitri,
I'm obviously missing something in my litterature  review. I did a new
MC simulation, with a much smaller number of observation points
(namely 3, to fit a straight line!!!). It turns out that the formula
you are advocating for is the best estimate of the standard deviation
of the parameters. Could you please explain why this fomula differs
from formulas (34) and (35) in
http://mathworld.wolfram.com/LeastSquaresFitting.html?

I hope I'm not bothering you too much. I really would like to
understand, so that we could write an accurate javadoc and possibly
rename the method appropriately.

Best regards,
Sébastien


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

Re: [math] Deprecating "guessParametersErrors"

2012/5/4 Gilles Sadowski :
> Hello.
>
>> I'm obviously missing something in my litterature  review. I did a new
>> MC simulation, with a much smaller number of observation points
>> (namely 3, to fit a straight line!!!). It turns out that the formula
>> you are advocating for is the best estimate of the standard deviation
>> of the parameters. Could you please explain why this fomula differs
>> from formulas (34) and (35) in
>> http://mathworld.wolfram.com/LeastSquaresFitting.html?
>
> Independently of the explanation to be provided by Dimitri, I think that
> there are code design arguments in favour of deprecating (and later,
> deleting) the "guessParametersErrors" method, as follows.
>
> In the context of the "optimization.general" package, one assumes that a
> Jacobian matrix is available. From there, the code in "AbstractLeastSquares"
> computes the covariance matrix, from which one can readily extract the
> "sigma".
> This can be done without computing the chi-square! [While, as you have
> probably noticed, the "guessParametersErrors" will not behave nicely if you
> don't call "updateResidualsAndCost()" beforehand.]
>
> For the class to be self-consistent, the story can end here: Any additional
> utilities can lead to wrong expectations from different types of users (as
> we've demonstrated here).
> Indeed, confidence intervals refer to additional variables (as Dimitri
> wrote: "By how much can a parameter change before the normalized chi2
> changes by ?"). Being able to answer those questions also
> involves the correlations between the parameters (cf. the plot I've attached
> to MATH-784), whereas "guessParametersErrors" does not take them into
> account.
>
>> I hope I'm not bothering you too much. I really would like to
>> understand, so that we could write an accurate javadoc and possibly
>> rename the method appropriately.
>
> For clarity's sake (design-wise), I propose to remove the
> "guessParametersErrors" method, and add a "getSigma" (as syntactic sugar).
>
I'm OK with that. As a first step, we deprecate it, and stipulate in
the javadoc that getSigma() should be used instead. We emphasize that
both methods do not provide exactly the same value. Similarly, I
propose that the javadoc of getSigma() states exactly what it returns
(namely, sqrt(cov[i][i])). Finally, do you think its worth calling
this method getSigmaParameters() in order to avoid confusion with sd
on the observations (which are implicitly assumed by the weights in
the chis-square)?

The tests I've recently added (NIST data) must be altered a bit, so
maybe I could take of the whole thing if you want. Then we could
consider that MATH-784 is resolved.

>
> If you want to dig further into the confidence interval issues in order to
> provide the related functionality (similar to, but not limited to, the
> current "guessParametersErrors"), I propose that that code be located in the
> "stat" package (where, by the way, some of the utilities might already
> exist!).
>
Maybe in the future I will dig into this problem, I find it very
interesting!!! For the time being, I'd rather concentrate on other
issues such as the current bugs reported in the distribution package
(been working a lot on this recently, even if I have not yet come up
with a patch...).
> What do you think?
>
>
> Best regards,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: [math] Deprecating "guessParametersErrors"

Hi Gilles,

>
> Before closing the issue, I'd like to commit the "test" I've attached to
> MATH-784. It's not really a "unit" test (as it doesn't "assert" anything
> yet) but it shows the meaning of "sigma".
> I've ended the class name with the string "TestValidation" so that it does
> not run by default with the "test" target.]
>
> Maybe I can add the assert statement (once the "getSigma" method is
> available).
>
> Is that OK for everyone?
>

Fine by me! Thanks for the interesting discussion!
Best regards,
Sébastien


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

Re: [math] Deprecating "guessParametersErrors"

Hi,
>
>>
>> Before closing the issue, I'd like to commit the "test" I've attached to
>> MATH-784. It's not really a "unit" test (as it doesn't "assert" anything
>> yet) but it shows the meaning of "sigma".
>> I've ended the class name with the string "TestValidation" so that it does
>> not run by default with the "test" target.]
>>
>> Maybe I can add the assert statement (once the "getSigma" method is
>> available).
>>
>> Is that OK for everyone?
>>
>
> Fine by me! Thanks for the interesting discussion!
> Best regards,
>
I've performed the agreed changes. Please review the javadoc comments.
Best regards,

Sébastien


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

Re: [math] Deprecating

2012-05-06 Thread Sébastien Brisard

Hi Dimitri, Ted,
thanks for both taking the time to provide such detailed and
interesting answers. I now need to digest all this.
>From what I already understand, I think that Gilles' suggestion, which
we finally settled for was probably the best one!
Best regards,
Sébastien


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

Re: [Math] Serializable (again)

Hello,

2012/5/8 Thomas Neidhart :
> On Tue, May 8, 2012 at 12:24 PM, Gilles Sadowski <
> gil...@harfang.homelinux.org> wrote:
>
>> > I am in favor of this second option: add Serializable were needed upon
>> > > request.
>> >
>> > +1
>>
>
> +1
>
>
>> > > In this case, the request seems fair as the class is mainly a
>> > > simple container for two values.
>> >
>> > But what ever the size/complexity, adding Serializable needs to be
>> > done carefully, and documented as necessary using @serial,
>> > @serialField, @serialData tags.
>>
>> > >
>> I am _not_ going to work on that. [Plenty of arguments given previously
>> saying exactly that: it must be done seriously. It is not so in CM. And
>> (IMHO) it is not necessary for CM to support "Serializable" (also argued
>> previously).]
>>
>> > Just adding "implements Serializable" is a bad idea.
>>
>> Agreed.
>>
>> So?  [I guess I'm not going to do anything myself on this issue.]
>>
>
> I think we should implement it where it is requested and makes sense (like
> results of a computation). But then in a proper way (should be defined in
> the developers guide or even the wiki as best practice to follow).
> For other cases we can just skip it, but please, do not remove Serializable
> just because you don't like it (as it happened in the case for the
> referenced issue).
>
> Thomas
I have mixed feelings on this issue. I guess that everyone agrees that
serialization must be done properly. Like Gilles, I'm not going to
work on this issue, probably for other reasons
1. I certainly do not have the required expertise (and do not have the
time to acquire it at the moment). Which makes me wonder about
maintenance of serializable classes. Committers like myself reviewing
other people's patches on serializable classes, might well be
innocently committing a patch which should not be. To prevent that, we
should make sure that we do have some "serialization referees" in the
CM community, which might overload these people.
2. I think there are many more urgent issues (only my opinion,
though), like the open issues about precision in the distribution
package, or the implementation of more special functions, which is a
quite time consuming, but necessary task.
Best regards,
Sébastien


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

[math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

Hi,
I've spent quite some time on MATH-753 [1], and I think I now have a
satisfactory solution.
The problem was to overcome the overflows which arise when computing
the density of the Gamma distribution for large values of the argument
and/or the scale parameter.

As I initially feared, what was proposed in the JIRA ticket leads to
high floating point errors. I adapted a method proposed in BOOST [2]
with acceptable errors. Meanwhile, I've also managed to improve the
accuracy of the computation of the density for the range of parameters
where the previous implementation was already working: in this range,
the accuracy *was* about 300 ulps, and is now 1-2 ulps! I think this
improvement is worth implementing.

The downside is that I need to expose the Lanczos implementation
internally used by o.a.c.m3.special.Gamma. This approximation is so
standard that I don't see it as a problem. I don't think that it
reveals too much of the Gamma internals, since the javadoc of
Gamma.logGamma states that it uses this approximation. So what I
propose is the addition of two methods in Gamma:
double getLanczosG(): returns the g constant
double getLanczos(double): returns the value of the Lanczos sum.

If you do not like this option, I can copy/paste the Lanczos
approximation in the GammaDistribution class. I'm adverse to the
latter option, as it leads to code duplication.

What do you think?

Best regards,
Sébastien

[1] https://issues.apache.org/jira/browse/MATH-753
[2]
http://www.boost.org/doc/libs/1_35_0/libs/math/doc/sf_and_dist/html/math_toolkit/special/sf_gamma/igamma.html,
formula (15)
[3] http://en.wikipedia.org/wiki/Lanczos_approximation, formula below
"[...] the sum is recast into the following form:"

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

Re: [math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

Hi Thomas,

2012/5/8 Thomas Neidhart :
> On 05/08/2012 05:26 PM, Sébastien Brisard wrote:
>> Hi,
>> I've spent quite some time on MATH-753 [1], and I think I now have a
>> satisfactory solution.
>> The problem was to overcome the overflows which arise when computing
>> the density of the Gamma distribution for large values of the argument
>> and/or the scale parameter.
>>
>> As I initially feared, what was proposed in the JIRA ticket leads to
>> high floating point errors. I adapted a method proposed in BOOST [2]
>> with acceptable errors. Meanwhile, I've also managed to improve the
>> accuracy of the computation of the density for the range of parameters
>> where the previous implementation was already working: in this range,
>> the accuracy *was* about 300 ulps, and is now 1-2 ulps! I think this
>> improvement is worth implementing.
>>
>> The downside is that I need to expose the Lanczos implementation
>> internally used by o.a.c.m3.special.Gamma. This approximation is so
>> standard that I don't see it as a problem. I don't think that it
>> reveals too much of the Gamma internals, since the javadoc of
>> Gamma.logGamma states that it uses this approximation. So what I
>> propose is the addition of two methods in Gamma:
>> double getLanczosG(): returns the g constant
>> double getLanczos(double): returns the value of the Lanczos sum.
>>
>> If you do not like this option, I can copy/paste the Lanczos
>> approximation in the GammaDistribution class. I'm adverse to the
>> latter option, as it leads to code duplication.
>>
>> What do you think?
>
> Hi Sébastien,
>
> this sounds pretty good.
>
> After looking at the current Gamma function implementation, it looks
> like it consists mainly of public static methods. Could you attach your
> changes to the issue so I can have a look at them?
>
I will prepare a patch and attach it to MATH-753. Thanks for proposing
to review it!

Sébastien
> Thanks,
>
> Thomas
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: [math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

Hi Gilles,

2012/5/8 Gilles Sadowski :
>> > I've spent quite some time on MATH-753 [1], and I think I now have a
>> > satisfactory solution.
>> > The problem was to overcome the overflows which arise when computing
>> > the density of the Gamma distribution for large values of the argument
>> > and/or the scale parameter.
>> >
>> > As I initially feared, what was proposed in the JIRA ticket leads to
>> > high floating point errors. I adapted a method proposed in BOOST [2]
>> > with acceptable errors. Meanwhile, I've also managed to improve the
>> > accuracy of the computation of the density for the range of parameters
>> > where the previous implementation was already working: in this range,
>> > the accuracy *was* about 300 ulps, and is now 1-2 ulps! I think this
>> > improvement is worth implementing.
>> >
>> > The downside is that I need to expose the Lanczos implementation
>> > internally used by o.a.c.m3.special.Gamma. This approximation is so
>> > standard that I don't see it as a problem. I don't think that it
>> > reveals too much of the Gamma internals, since the javadoc of
>> > Gamma.logGamma states that it uses this approximation. So what I
>> > propose is the addition of two methods in Gamma:
>> > double getLanczosG(): returns the g constant
>> > double getLanczos(double): returns the value of the Lanczos sum.
>
> Is that something useful only inside the "Gamma" class?
> If not, maybe that it can become a stand-alone class, or a method in one of
> the "utility" classes (in package "util").
>
This is something which is in very close connection to the Gamma
function. I think it should stay in the Gamma class.

>> >
>> > If you do not like this option, I can copy/paste the Lanczos
>> > approximation in the GammaDistribution class. I'm adverse to the
>> > latter option, as it leads to code duplication.
>
> No "copy/paste" for me, thank you. :-)
>
That's what I thought...
Sébastien
>
> Best,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: [math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

Hi,
the patch for the proposed modification has been attached to the
MATH-753 ticket. Thanks for your comments!
Best regards,
Sébastien


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

Re: [math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

Hi Thomas,

>
>> Hi,
>> the patch for the proposed modification has been attached to the
>> MATH-753 ticket. Thanks for your comments!
>>
>
> What I have seen in the patch, you remove duplicated code by using the
> already existing lanczos function, and create an additional function for
> the constant.
>
In fact, I've realized that I have accidentally committed the
lanczos() function in r1334315, which has nothing to do with this
matter (that revision was about MATH-784). In fact, double
lanczos(double) is indeed new.
I will try to revert the change in Gamma in r1334315, and commit it in
a new revision, once we all agree that double lanczos(double) can be
exposed.

So the question is: do you like this function lanczos (and its
Javadoc) or not. Do you agree with this method being public?

>
> I guess just creating a public final static constant for the LANCZOS_G
> value would be better?
>
Yes, you're right, it is better...

Thanks
> Thomas


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

Re: [math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

OK, I've cleaned up my mess, and attached the true patch to MATH-753.
Sorry about that.

Sébastien


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

Re: [math] Exposing the Lanczos approximation in o.a.c.m3.special.Gamma?

2012/5/9 Gilles Sadowski :
>> >> the patch for the proposed modification has been attached to the
>> >> MATH-753 ticket. Thanks for your comments!
>> >>
>> >
>> > What I have seen in the patch, you remove duplicated code by using the
>> > already existing lanczos function, and create an additional function for
>> > the constant.
>> >
>> In fact, I've realized that I have accidentally committed the
>> lanczos() function in r1334315, which has nothing to do with this
>> matter (that revision was about MATH-784). In fact, double
>> lanczos(double) is indeed new.
>> I will try to revert the change in Gamma in r1334315, and commit it in
>> a new revision, once we all agree that double lanczos(double) can be
>> exposed.
>>
>> So the question is: do you like this function lanczos (and its
>> Javadoc) or not. Do you agree with this method being public?
>
> "public" is preferrable to duplicate code (if that's the question...).
>
I guess it is the question... Code duplication vs. exposing a little
bit of the internals of Gamma.
I will commit the patch.

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


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

Re: [Math] Serializable (again)

Hi James,

2012/5/9 James Carman :
> On Wed, May 9, 2012 at 3:03 AM, Luc Maisonobe  wrote:
>>> Is there such a thing as short-term serialization?
>>
>> Yes, of course, and this what many people need. This is what James
>> called marshalling/unmarshalling. This is a standard way to communicate
>> between some applications that share a common basis. Of course [math] is
>> not a distributed computing environment, but it should not prevent
>> people from being used in a distributed environment.
>>
>
> For long-term serialization needs, I would suggest instead that we
> introduce (perhaps it's there already, I'm not familiar with the
> entire codebase) exporters/importers which support some
> industry-standard file format(s) (such as those used by R for
> example).  Now, I would only suggest we introduce these where it makes
> sense, such as for larger data structures like matrices.  For
> something with a few fields, not so much.
>
Setting aside the discussion on serialization, this would probably be
a nice extension. The problem is to find a good (as in "widely used")
standard. Do you think the file formats used by R are good candidates
(meaning: used outside the R community)? Are these file format indeed
standardized, with precise specifications?
I've heard about HDF as well, but I have not investigated.
Sébastien


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

Re: [Math] Serializable (again)

Hi James,

2012/5/9 James Carman :
> On Wed, May 9, 2012 at 7:10 AM, Sébastien Brisard
>  wrote:
>> Setting aside the discussion on serialization, this would probably be
>> a nice extension. The problem is to find a good (as in "widely used")
>> standard. Do you think the file formats used by R are good candidates
>> (meaning: used outside the R community)? Are these file format indeed
>> standardized, with precise specifications?
>> I've heard about HDF as well, but I have not investigated.
>
> I only chose the R stuff because in my opinion, the file formats are
> very easy.  From my recollection, R supports simple text-based input
> (tab-delimited) very well.  I believe it also has "object" files that
> it can spit out, but I don't know if we want to go that far.
>
That's what I was thinking of: binary file formats.

>
> I'm no
> expert on the math software industry (so much for that math degree).
> I would leave that up to folks from the CM project to decide.
> However, I would think that if you support a textual, delimited file
> format, most math libraries would be able to consume it (and produce
> files that CM could consume as well).
>
We should be careful not rewriting Commons-CSV...
Best regards,
Sébastien


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

[math] Possible confusions in GammaDistribution

2012-05-15 Thread Sébastien Brisard

Hi,
while working on MATH-753, I found that some methods
o.a.c.m3.distribution.GammaDistribution were ill-named. Indeed, the
class javadoc explicitly refers to the Wikipedia page. In this page,
the shape parameter is named alpha and the scale parameter is 1 / beta
(inverse of beta), while in the current implementation, the shape
parameter is named alpha, and the scale parameter is named beta (not 1
/ beta !!!). This might be confusing. Looking at other web pages, I
see there is apparently no consensus on the naming of shape and scale
parameters. The problem lies with the accessors, which are
double getAlpha()
double getBeta()

I think these accessors can lead to confusion. I would therefore
propose to deprecate them, and replace them with
double getShape()
double getScale()

What do you think ?

Sébastien


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

Re: [continuum] BUILD FAILURE: Apache Commons - Commons Math - Default Maven 2 Build Definition (Java 1.5)

2012-05-16 Thread Sébastien Brisard

Hi,

>
> Online report : 
> http://vmbuild.apache.org/continuum/buildResult.action?buildId=22195&projectId=97
>
...
>
>  java.lang.AssertionError: chi-square test -- will fail about 1 in 1000 times
>
This test has failed twice in the past week or so... It might be due
to the changes I've made to Gamma or GammaDistribution... I will
investigate.

Best regards,
Sébastien


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

Re: [math] numerical stability problems of continued fraction

2012-05-20 Thread Sébastien Brisard

Hi Thomas,
>
> This seems to solve all the described problems, but the question remains
> if this can be included without problems into CM. The algorithm
> description in NR is not in source code but on an higher level.
>
Looking at the relevant chapter in NR, I don't think there is any
license issue (no code provided in NR).
However, if you really want to play it safe, the paper by Thompson is
available here
http://www.fresco.org.uk/papers/Thompson-JCP64p490.pdf
I guess downloading it is OK, as www.fresco.org.uk seems to be
Thompson's own website.
What you could do is conform closely to Thompson's notation, instead
of NR's notation.

Sébastien


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

Re: Git support?

2012-05-20 Thread Sébastien Brisard

Hi,
>
> I have been, um, "absent" for a while, but am going to try to get
> active again.  For now, just catching up on stuff.  (I have to say,
> my being "away" seems to have helped some things move along :)
> Great work, guys!)  I am a Git ignoramus, so will be a good test
> case for how easy it is for non-Git ppl to manage through the
> transition.  If other [math] ppl are OK with it, I would say go for
> it, Luc!
>
> Phil
>
So good to here from you again!!!
As for Git + CM3, I'm OK with that (wanted to give Git a go anyway,
will *have* to do it, then).
Sébastien


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

[math] LocalizedFormats.ALPHA and BETA no longer used

Hi,
commit 1342404 solves MATH-791. It turns out that
LocalizedFormats.ALPHA and BETA are no longer used. I wanted to remove
them, but that breaks the compatibility. How should we do that
cleanly?

Best regards,
Sébastien


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

Re: [math] LocalizedFormats.ALPHA and BETA no longer used

Hi Luc,

2012/5/24 Luc Maisonobe :
> Le 24/05/2012 21:27, Sébastien Brisard a écrit :
>> Hi,
>> commit 1342404 solves MATH-791. It turns out that
>> LocalizedFormats.ALPHA and BETA are no longer used. I wanted to remove
>> them, but that breaks the compatibility. How should we do that
>> cleanly?
>
> Just remove the entries from the enum and temove the corresponding entry
> in the translation properties file.
>
I'm sorry, I should have mentioned that I've tried that, but it breaks
compatibility (these field are public).
Can we "deprecate" a field?
Best regards,
Sébastien


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

[math] visitor pattern for vectors

Hi,
MATH-765 turns out to be too ambitious [1], but one of the points I
raised was deemed worth implementing.

{quote}
My first suggestion would be on the visitor design pattern vs.
map(UnivariateFunction). The former is specified in the RealMatrix
interface, the latter is specified in the RealVector abstract class. I
think both concepts are similar, and both are useful:
  - visitors know about the cell they are visiting,
  - map() doesn't.
Maybe it would be nice as a first step to unify these concepts. Two
options there
1. Specify both in both interfaces,
2. Specify only the visitor design pattern, and create a factory which
would return a visitor from a UnivariateFunction (ignoring the indices
of the current cell).
{quote}


Gilles wrote that he liked this proposal, and favored option #2. What
do others think?

Best regards,

Sébastien

[1] http://markmail.org/thread/gvorzl6cvgibkjun


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

Re: [math] LocalizedFormats.ALPHA and BETA no longer used

Hi,
thanks for your answer!
>
> You can deprecate a field.
>
Good to know, I wasn't too sure it makes sense.

>
> However, if a public field is not part of the user API - e.g. it's
> only public to allow access within CM code - then breaking compat is
> not a problem, because users should never reference the field.
>
It seems to me that we are exactly in this situation. So if noone
objects I'd like to do that. However, Clirr will complain... How can I
avoid this?

Best regards,
Sébastien


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

Re: [math] visitor pattern for vectors

Hello again,

>
> MATH-765 turns out to be too ambitious [1], but one of the points I
> raised was deemed worth implementing.
>
> {quote}
> My first suggestion would be on the visitor design pattern vs.
> map(UnivariateFunction). The former is specified in the RealMatrix
> interface, the latter is specified in the RealVector abstract class. I
> think both concepts are similar, and both are useful:
>  - visitors know about the cell they are visiting,
>  - map() doesn't.
> Maybe it would be nice as a first step to unify these concepts. Two
> options there
> 1. Specify both in both interfaces,
> 2. Specify only the visitor design pattern, and create a factory which
> would return a visitor from a UnivariateFunction (ignoring the indices
> of the current cell).
> {quote}
>
>
> Gilles wrote that he liked this proposal, and favored option #2. What
> do others think?
>
> Best regards,
>
> Sébastien
>
> [1] http://markmail.org/thread/gvorzl6cvgibkjun
>
On second thoughts, I think that option #2 is preferrable for sparse
vectors. Indeed, in the case of map(), the function to be mapped needs
only be called once for the default value of the entries which are not
stored. For the visitor pattern, as the index of the entry might
affect the behavior of the visitor, this optimization cannot be done.
BTW, I see that the value of the unstored entries in sparse
vectors/matrices is zero. This might be a bit restrictive. For
example, if x is a sparse vector, then x.map(cos) should also (in my
opinion) be a sparse vector. Maybe we should think about this
extension?

Best regards,
Sébastien


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

Re: [math] LocalizedFormats.ALPHA and BETA no longer used

2012-05-27 Thread Sébastien Brisard

Hi,

2012/5/27 Luc Maisonobe :
> Le 27/05/2012 16:25, Jochen Wiedmann a écrit :
>> On Thu, May 24, 2012 at 8:41 PM, Luc Maisonobe  wrote:
>>> Le 24/05/2012 21:27, Sébastien Brisard a écrit :
>>>> Hi,
>>>> commit 1342404 solves MATH-791. It turns out that
>>>> LocalizedFormats.ALPHA and BETA are no longer used. I wanted to remove
>>>> them, but that breaks the compatibility. How should we do that
>>>> cleanly?
>>>
>>> Just remove the entries from the enum and temove the corresponding entry
>>> in the translation properties file.
>>
>> Why remove them at all? I understand the desire, it meets my sense for
>> clean code. But I learned to fight against these desires from time to
>> time.
>
> These parameters are mainly for internal use, we don't consider them as
> part of the public API. As we remove their internal use, it seems good
> to be consistent and remove everything at once.
>
I fully agree.

>
> Do you have an idea about another use that could justify we retain them ?
>
It's probably just as well to restore these constants if we find a use
for them at a later point (that would be trivial).

So, do we agree that these constants should go? How should I proceed?
The clean way (deprecation in 3.1, and complete removal in 4.0), or
the brutal way (complete removal in 3.1) ?

Best regards,
Sébastien


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

[math] SparseRealVectorTest, SparseRealMatrixTest, SparseFieldMatrixTest, SparseFieldVectorTest

Hi,
is there a reason why these classes are called SparseXxxTest, instead
of OpenMapXxxTest? Am I missing something?
Sébastien


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

Re: svn commit: r1343163 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/ test/java/org/apache/commons/math3/linear/

Hi Gilles,

>
> Why do you call "valueOf"?
>
I have to say I do not like implicit conversions, that's why I tend to
always use Integer.valueOf and the likes. However, if you think that
it leads to less readable code, I'm OK with your way of seeing things.

Best regards,
Sébastien


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

Re: [continuum] BUILD FAILURE: Apache Commons - Commons Math - Default Maven 2 Build Definition (Java 1.5)

Hi,

2012/5/28 Continuum@vmbuild :
> Online report : 
> http://vmbuild.apache.org/continuum/buildResult.action?buildId=22678&projectId=97
>
> Build statistics:
>  State: Failed
>  Previous State: Ok
>  Started at: Mon 28 May 2012 09:39:04 +
>  Finished at: Mon 28 May 2012 09:42:01 +
>  Total time: 2m 57s
>  Build Trigger: Schedule
>  Build Number: 825
>  Exit code: 1
>  Building machine hostname: vmbuild
>  Operating system : Linux(unknown)
>  Java Home version :
>          java version "1.6.0_30"
>          Java(TM) SE Runtime Environment (build 1.6.0_30-b12)
>          Java HotSpot(TM) 64-Bit Server VM (build 20.5-b03, mixed mode)
>
>  Builder version :
>          Apache Maven 2.2.1 (r801777; 2009-08-06 19:16:01+)
>          Java version: 1.6.0_30
>          Java home: /usr/lib/jvm/jdk1.6.0_30/jre
>          Default locale: en_US, platform encoding: ANSI_X3.4-1968
>          OS name: "linux" version: "2.6.32-31-server" arch: "amd64" Family: 
> "unix"
>
> 
> SCM Changes:
> 
> Changed: celestin @ Mon 28 May 2012 08:31:13 +
> Comment: MATH-792:
>  - various methods to visit the entries of a RealVector (entries are 
> unmodified),
>  - default implementation in RealVector abstract class,
>  - unit tests in abstract RealVectorAbstractClass,
>  - all XxxRealVectorTest classes now extend RealVectorAbstractClass.
> Files changed:
>  /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>  ( 1343163 )
>  /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVectorPreservingVisitor.java
>  ( 1343163 )
>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/ArrayRealVectorTest.java
>  ( 1343163 )
>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/RealVectorAbstractTest.java
>  ( 1343163 )
>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/RealVectorTest.java
>  ( 1343163 )
>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
>  ( 1343163 )
>
> 
> Dependencies Changes:
> 
> No dependencies changed
>
>
> 
> Build Definition:
> 
> POM filename: pom.xml
> Goals: clean deploy
> Arguments: --batch-mode -Pjava-1.6
> Build Fresh: false
> Always Build: false
> Default Build Definition: true
> Schedule: COMMONS_SCHEDULE
> Profile Name: Maven 2.2.1
> Description: Default Maven 2 Build Definition (Java 1.5)
>
> 
> Test Summary:
> 
> Tests: 3598
> Failures: 0
> Errors: 2
> Success Rate: 99
> Total time: 123.96295
>
>
>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
I'm the culprit. The test in question uses reflexion a lot, so adding
new methods to RealVector directly affects the test. Will look into
it.
Sorry about that, I promise I will clean it ASAP.
Best regards,
Sébastien


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

Re: [math] SparseRealVectorTest, SparseRealMatrixTest, SparseFieldMatrixTest, SparseFieldVectorTest

Hi,

2012/5/28 Thomas Neidhart :
> On 05/28/2012 11:23 AM, Luc Maisonobe wrote:
>> Le 28/05/2012 09:42, Sébastien Brisard a écrit :
>>> Hi,
>>> is there a reason why these classes are called SparseXxxTest, instead
>>> of OpenMapXxxTest? Am I missing something?
>>
>> I don't remember the reason, it may either be that we renamed the
>> library class and forgot to rename the test classes, or we intended to
>> share base test class accross several different implementations and did
>> not do it.
>>
I'm currently looking into the tests of all vectors implementations,
and there is *a lot* of duplicate code. I will probably factor out
some of this code into the newly created RealVectorAbstractTest (which
I've set up for the sake of testing the various implementations of the
visitor pattern).

>> Nevertheless, they could be renamed for consistency. As they are only
>> test classes, there are no compatibility issues there.
>>
>> Thanks for identifying this.
>
> fyi: there is already an issue about this MATH-625
>
> Thomas
>
Thomas, you are a living encyclopedia! Thanks for the hint, I you
want, I can take care of that ticket.

Best regards,
Sébastien


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

Re: [continuum] BUILD FAILURE: Apache Commons - Commons Math - Default Maven 2 Build Definition (Java 1.5)

Hi again,
should be fixed in r1343217.
The fix is not fully satisfactory, as the visitor pattern gets
untested on unmodifiable views of vectors. However, I intend to
refactor the tests for all vector classes (introducing a common
RealVectorAbstractTest base class), and am hoping that this base class
can be used for unmodifiable views as well.
Sébastien

2012/5/28 Sébastien Brisard :
> Hi,
>
> 2012/5/28 Continuum@vmbuild :
>> Online report : 
>> http://vmbuild.apache.org/continuum/buildResult.action?buildId=22678&projectId=97
>>
>> Build statistics:
>>  State: Failed
>>  Previous State: Ok
>>  Started at: Mon 28 May 2012 09:39:04 +
>>  Finished at: Mon 28 May 2012 09:42:01 +
>>  Total time: 2m 57s
>>  Build Trigger: Schedule
>>  Build Number: 825
>>  Exit code: 1
>>  Building machine hostname: vmbuild
>>  Operating system : Linux(unknown)
>>  Java Home version :
>>          java version "1.6.0_30"
>>          Java(TM) SE Runtime Environment (build 1.6.0_30-b12)
>>          Java HotSpot(TM) 64-Bit Server VM (build 20.5-b03, mixed mode)
>>
>>  Builder version :
>>          Apache Maven 2.2.1 (r801777; 2009-08-06 19:16:01+)
>>          Java version: 1.6.0_30
>>          Java home: /usr/lib/jvm/jdk1.6.0_30/jre
>>          Default locale: en_US, platform encoding: ANSI_X3.4-1968
>>          OS name: "linux" version: "2.6.32-31-server" arch: "amd64" Family: 
>> "unix"
>>
>> 
>> SCM Changes:
>> 
>> Changed: celestin @ Mon 28 May 2012 08:31:13 +
>> Comment: MATH-792:
>>  - various methods to visit the entries of a RealVector (entries are 
>> unmodified),
>>  - default implementation in RealVector abstract class,
>>  - unit tests in abstract RealVectorAbstractClass,
>>  - all XxxRealVectorTest classes now extend RealVectorAbstractClass.
>> Files changed:
>>  /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>  ( 1343163 )
>>  /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVectorPreservingVisitor.java
>>  ( 1343163 )
>>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/ArrayRealVectorTest.java
>>  ( 1343163 )
>>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/RealVectorAbstractTest.java
>>  ( 1343163 )
>>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/RealVectorTest.java
>>  ( 1343163 )
>>  /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
>>  ( 1343163 )
>>
>> 
>> Dependencies Changes:
>> 
>> No dependencies changed
>>
>>
>> 
>> Build Definition:
>> 
>> POM filename: pom.xml
>> Goals: clean deploy
>> Arguments: --batch-mode -Pjava-1.6
>> Build Fresh: false
>> Always Build: false
>> Default Build Definition: true
>> Schedule: COMMONS_SCHEDULE
>> Profile Name: Maven 2.2.1
>> Description: Default Maven 2 Build Definition (Java 1.5)
>>
>> 
>> Test Summary:
>> 
>> Tests: 3598
>> Failures: 0
>> Errors: 2
>> Success Rate: 99
>> Total time: 123.96295
>>
>>
>>
>>
>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> For additional commands, e-mail: dev-h...@commons.apache.org
>>
> I'm the culprit. The test in question uses reflexion a lot, so adding
> new methods to RealVector directly affects the test. Will look into
> it.
> Sorry about that, I promise I will clean it ASAP.
> Best regards,
> Sébastien


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

Re: svn commit: r1343163 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/ test/java/org/apache/commons/math3/linear/

Hi Gilles,

2012/5/28 Gilles Sadowski :
> Hello.
>
>>
>> >
>> > Why do you call "valueOf"?
>> >
>> I have to say I do not like implicit conversions, that's why I tend to
>> always use Integer.valueOf and the likes.
>
> Why?
>
This is going to get "philosophical": I would not dare to claim that
I'm holding *the* truth, this is only my way of seeing things. I do
not like things to happen implicitely, because I think that it opens a
door to errors (this would not be true of professional programmers,
but remember that I work in an environment where people are not real
computer scientists... so my philosophy is "close as many doors as you
can"...). I even think that J. Bloch has a nice example of potential
issues with auto-boxing. However, I agree with you: calling valueOf in
this context (that is: building a new exception) is certainly
far-fetched.

That was the initial reason why I moved from C++ to Java ten years
ago: too many things happened "behind my back". Again, I'm not saying
that C++ is evil. I'm just saying that I did not have the background
with C++ to be fully aware of these implicit assumptions, and their
potential consequences.

I'm pretty sure I will not convince you on this. I hope I have at
least convinced you that I have a good reason to do it this way :-)

>> However, if you think that
>> it leads to less readable code, I'm OK with your way of seeing things.
>
> Good! ;-)
>
> Gilles
>
As I said, I will remove these explicit boxings (but be warned: I've
used valueOf() in the past, and do apologize for any unwanted
occurence...).

Best regards,
Sébastien


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

Re: svn commit: r1343163 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/ test/java/org/apache/commons/math3/linear/

Hello again,

2012/5/28 sebb :
> On 28 May 2012 13:28, Sébastien Brisard  wrote:
>> Hi Gilles,
>>
>> 2012/5/28 Gilles Sadowski :
>>> Hello.
>>>
>>>>
>>>> >
>>>> > Why do you call "valueOf"?
>>>> >
>>>> I have to say I do not like implicit conversions, that's why I tend to
>>>> always use Integer.valueOf and the likes.
>>>
>>> Why?
>>>
>> This is going to get "philosophical": I would not dare to claim that
>> I'm holding *the* truth, this is only my way of seeing things. I do
>> not like things to happen implicitely, because I think that it opens a
>> door to errors (this would not be true of professional programmers,
>> but remember that I work in an environment where people are not real
>> computer scientists... so my philosophy is "close as many doors as you
>> can"...). I even think that J. Bloch has a nice example of potential
>> issues with auto-boxing. However, I agree with you: calling valueOf in
>> this context (that is: building a new exception) is certainly
>> far-fetched.
>>
>> That was the initial reason why I moved from C++ to Java ten years
>> ago: too many things happened "behind my back". Again, I'm not saying
>> that C++ is evil. I'm just saying that I did not have the background
>> with C++ to be fully aware of these implicit assumptions, and their
>> potential consequences.
>>
>> I'm pretty sure I will not convince you on this. I hope I have at
>> least convinced you that I have a good reason to do it this way :-)
>>
>
> I agree with you - making boxing explicit is better.
>
> The reasoning is that implicit boxing/unboxing can hide ineffiicient
> conversions as well as subtle bugs.
> I've seen code which treats the same item variously as a Integer and
> an int for no good reason.
> The compiler warnings allow the careful programmer to investigate and
> choose one or the other.
>
> Yes, the code is a bit longer, but it clearly shows that the
> boxing/unboxing was intended, and not an accident of compiler
> "helpfulness".
>
>>>> However, if you think that
>>>> it leads to less readable code, I'm OK with your way of seeing things.
>>>
>>> Good! ;-)
>>>
>>> Gilles
>>>
>> As I said, I will remove these explicit boxings (but be warned: I've
>> used valueOf() in the past, and do apologize for any unwanted
>> occurence...).
>>
>> Best regards,
>> Sébastien
>>
Wowww!
Who would have thought that this would raise so many answers!
While I also think explicit conversion is better, I think we can all
agree that Gilles has a point for this specific case

throw new OutOfRangeException(LocalizedFormats.INDEX,
  Integer.valueOf(start),
  Integer.valueOf(0),
  Integer.valueOf(dim - 1));

is a bit far-fetched, since the code ends right there. I think we can
safely follow Gilles' advice, here.
Best regards,
Sébastien


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

Re: svn commit: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java

2012/5/28 sebb :
> On 28 May 2012 16:39,   wrote:
>> Author: celestin
>> Date: Mon May 28 15:39:57 2012
>> New Revision: 1343293
>>
>> URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
>> Log:
>> Removed explicit conversion from int to Integer.
>
> -1; I don't think we had concensus on removing it.
>
>>
>> Modified:
>>    
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>
>> Modified: 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>> URL: 
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
>> ==
>> --- 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>  (original)
>> +++ 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>  Mon May 28 15:39:57 2012
>> @@ -202,23 +202,17 @@ public abstract class RealVector {
>>     protected void checkIndices(final int start, final int end) {
>>         final int dim = getDimension();
>>         if ((start < 0) || (start >= dim)) {
>> -            throw new OutOfRangeException(LocalizedFormats.INDEX,
>> -                                          Integer.valueOf(start),
>> -                                          Integer.valueOf(0),
>> -                                          Integer.valueOf(dim - 1));
>> +            throw new OutOfRangeException(LocalizedFormats.INDEX, start, 0,
>> +                                          dim - 1);
>>         }
>>         if ((end < 0) || (end >= dim)) {
>> -            throw new OutOfRangeException(LocalizedFormats.INDEX,
>> -                                          Integer.valueOf(end),
>> -                                          Integer.valueOf(0),
>> -                                          Integer.valueOf(dim - 1));
>> +            throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
>> +                                          dim - 1);
>>         }
>> -        if (end < start){
>> +        if (end < start) {
>>             // TODO Use more specific error message
>>             throw new 
>> NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
>> -                                                Integer.valueOf(end),
>> -                                                Integer.valueOf(start),
>> -                                                false);
>> +                                                end, start, false);
>>         }
>>     }
>>
>>
>>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
That's true we didn't. However (see my last message), this is really
of little consequence, and I didn't really want to get into a new
"to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
I was the one who initially defended explicit conversion as opposed to
autoboxing, so you need not convince me. I guess most IDEs would
signal this auto-boxing. Can we assume on this specific case that we
are adults who know what they are doing? Again, this is just to build
the message of an exception, so the code virtually ends here.
I'm not familiar with -1ing a commit, I think I have to revert it
(could you confirm on that?). I'm happy to, but please find a
consensus with Gilles then, and I'm very happy to line up with the
outcome of your discussions.

Kind regards,
Sébastien


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

Re: svn commit: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java

2012/5/28 sebb :
> On 28 May 2012 17:12, Sébastien Brisard  wrote:
>> 2012/5/28 sebb :
>>> On 28 May 2012 16:39,   wrote:
>>>> Author: celestin
>>>> Date: Mon May 28 15:39:57 2012
>>>> New Revision: 1343293
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
>>>> Log:
>>>> Removed explicit conversion from int to Integer.
>>>
>>> -1; I don't think we had concensus on removing it.
>>>
>>>>
>>>> Modified:
>>>>    
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>>
>>>> Modified: 
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>> URL: 
>>>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
>>>> ==
>>>> --- 
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>>  (original)
>>>> +++ 
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>>  Mon May 28 15:39:57 2012
>>>> @@ -202,23 +202,17 @@ public abstract class RealVector {
>>>>     protected void checkIndices(final int start, final int end) {
>>>>         final int dim = getDimension();
>>>>         if ((start < 0) || (start >= dim)) {
>>>> -            throw new OutOfRangeException(LocalizedFormats.INDEX,
>>>> -                                          Integer.valueOf(start),
>>>> -                                          Integer.valueOf(0),
>>>> -                                          Integer.valueOf(dim - 1));
>>>> +            throw new OutOfRangeException(LocalizedFormats.INDEX, start, 
>>>> 0,
>>>> +                                          dim - 1);
>>>>         }
>>>>         if ((end < 0) || (end >= dim)) {
>>>> -            throw new OutOfRangeException(LocalizedFormats.INDEX,
>>>> -                                          Integer.valueOf(end),
>>>> -                                          Integer.valueOf(0),
>>>> -                                          Integer.valueOf(dim - 1));
>>>> +            throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
>>>> +                                          dim - 1);
>>>>         }
>>>> -        if (end < start){
>>>> +        if (end < start) {
>>>>             // TODO Use more specific error message
>>>>             throw new 
>>>> NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
>>>> -                                                Integer.valueOf(end),
>>>> -                                                Integer.valueOf(start),
>>>> -                                                false);
>>>> +                                                end, start, false);
>>>>         }
>>>>     }
>>>>
>>>>
>>>>
>>>
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>>> For additional commands, e-mail: dev-h...@commons.apache.org
>>>
>> That's true we didn't. However (see my last message), this is really
>> of little consequence, and I didn't really want to get into a new
>> "to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
>> I was the one who initially defended explicit conversion as opposed to
>> autoboxing, so you need not convince me. I guess most IDEs would
>> signal this auto-boxing.
>
> Depends on the setting.
>
>> Can we assume on this specific case that we
>> are adults who know what they are doing?
>
> The way to signal that is either to use explicit boxing, or use
> @SuppressWarnings("boxing") for that method.
>
> Otherwise, at some point someone else is going to wonder whether the
> boxing is intentional or not.
>
>> Again, this is just to build
>> the message of an exception, so the code virtually ends here.
>> I'm not familiar with -1ing a commit, I think I have to revert it
>> (could you confirm on that?).
>
> The committer either has to

Re: svn commit: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java

Hello,
>
> I don't see why there should be a consensus to remove something that was
> never there in the first place. The vast majority of such exception
> instantiations do not perform explicit conversion; so I'd be inclined to
> think that there should be a consensus to change that.
>
>
> Best regards,
> Gilles
>
while I agree with you, I do not really know what to do: am I to
ignore this -1 on one of my commits? This goes against the rules.
In its latest state, the code I've committed uses auto-boxing.

Best regards,
Sébastien


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

[math] Duplicate code in ArrayRealVectorTest, RealVectorTest, SparseRealVectorTest

2012-05-29 Thread Sébastien Brisard

Dear all,
while working on MATH-792 [1], I've noticed that the above mentioned
tests contain a lot of duplicate code. Besides, some of the unit tests
are no longer (in my opinion) unit tests, since they test many
different functionalities.
I would be tempted to refactor these tests. I've recently implemented
RealVectorAbstractTest for testing the various implementations of the
visitor pattern. That's probably the place where the code should be
factored out.
Besides readability, the benefit would be use of implementation of
other kinds of vector. The immediate benefit for me would be for the
implementation of views of vectors (which have been discussed some
time ago on the ML): testing would be almost trivial (I hope). I think
this refactoring would also lead to a natural resolution of MATH-625
[2].

It could be argued that this is counter-productive, as I would be
doing again what has already been done (and works!). I do not know if
we intend to release 3.1 soon, in which case this proposal might be
postponed.

What do you think?

Best regards,
Sébastien

[1] https://issues.apache.org/jira/browse/MATH-792
[2] https://issues.apache.org/jira/browse/MATH-625


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

Re: [math] Duplicate code in ArrayRealVectorTest, RealVectorTest, SparseRealVectorTest

2012-05-30 Thread Sébastien Brisard

Hello,
assuming that no answer meant no (strong) opposition, I took the
liberty to create MATH-795 [1]. This refactoring has already revealed
a problem (see r1344570 and r1344571).

Best regards,

Sébastien

[1] https://issues.apache.org/jira/browse/MATH-795

2012/5/30 Sébastien Brisard :
> Dear all,
> while working on MATH-792 [1], I've noticed that the above mentioned
> tests contain a lot of duplicate code. Besides, some of the unit tests
> are no longer (in my opinion) unit tests, since they test many
> different functionalities.
> I would be tempted to refactor these tests. I've recently implemented
> RealVectorAbstractTest for testing the various implementations of the
> visitor pattern. That's probably the place where the code should be
> factored out.
> Besides readability, the benefit would be use of implementation of
> other kinds of vector. The immediate benefit for me would be for the
> implementation of views of vectors (which have been discussed some
> time ago on the ML): testing would be almost trivial (I hope). I think
> this refactoring would also lead to a natural resolution of MATH-625
> [2].
>
> It could be argued that this is counter-productive, as I would be
> doing again what has already been done (and works!). I do not know if
> we intend to release 3.1 soon, in which case this proposal might be
> postponed.
>
> What do you think?
>
> Best regards,
> Sébastien
>
> [1] https://issues.apache.org/jira/browse/MATH-792
> [2] https://issues.apache.org/jira/browse/MATH-625


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

Re: [math] LocalizedFormats.ALPHA and BETA no longer used

2012-05-30 Thread Sébastien Brisard

Hi,

2012/5/27 Sébastien Brisard :
> Hi,
>
> 2012/5/27 Luc Maisonobe :
>> Le 27/05/2012 16:25, Jochen Wiedmann a écrit :
>>> On Thu, May 24, 2012 at 8:41 PM, Luc Maisonobe  
>>> wrote:
>>>> Le 24/05/2012 21:27, Sébastien Brisard a écrit :
>>>>> Hi,
>>>>> commit 1342404 solves MATH-791. It turns out that
>>>>> LocalizedFormats.ALPHA and BETA are no longer used. I wanted to remove
>>>>> them, but that breaks the compatibility. How should we do that
>>>>> cleanly?
>>>>
>>>> Just remove the entries from the enum and temove the corresponding entry
>>>> in the translation properties file.
>>>
>>> Why remove them at all? I understand the desire, it meets my sense for
>>> clean code. But I learned to fight against these desires from time to
>>> time.
>>
>> These parameters are mainly for internal use, we don't consider them as
>> part of the public API. As we remove their internal use, it seems good
>> to be consistent and remove everything at once.
>>
> I fully agree.
>
>>
>> Do you have an idea about another use that could justify we retain them ?
>>
> It's probably just as well to restore these constants if we find a use
> for them at a later point (that would be trivial).
>
> So, do we agree that these constants should go? How should I proceed?
> The clean way (deprecation in 3.1, and complete removal in 4.0), or
> the brutal way (complete removal in 3.1) ?
>
> Best regards,
> Sébastien

I've created MATH-796 for this issue. My favoured option would be
complete removal in 3.1, together with explanations in the release
notes (as suggested by sebb) to account for clirr's complaints.
Sébastien


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

Re: [Math] About MATH-798

2012-06-02 Thread Sébastien Brisard

Hi Gilles,

2012/6/2 Luc Maisonobe :
> Le 02/06/2012 01:55, Gilles Sadowski a écrit :
>> Hello.
>
> Hi Gilles,
>
>>
>> Do you know the rationale behind the (very) small values for
>>  DEFAULT_RELATIVE_THRESHOLD (set to 100 * Precision.EPSILON)
>>  DEFAULT_ABSOLUTE_THRESHOLD (set to 100 * Precision.SAFE_MIN)
>> in "AbstractConvergenceChecker"?
>> [I created this class as part of a refactoring, but the values were carried
>> over from whatever class contained that functionality before.]
>>
>> With those values, the "GaussNewtonOptimizer" fails to find the solution but
>> if one changes the threshold to either
>>  1e3 * Precision.EPSILON (for the relative threshold)
>> or
>>  1e281 * Precision.SAFE_MIN (for the absolute threshold)
>> the solution is found in 4 evaluations!
>>
>> As I wrote on the user ML, the thresholds were too stringent.
>> Are the current values really suitable as defaults (in other part of the
>> library, similar defaults are much larger)?
>> [I even think that there shouldn't be any defaults, so that users are
>> actually aware that the thresholds are problem-dependent and sometimes even
>> optimizer-dependent. My preference would thus be to deprecate the default
>> constructor in all the checker classes.]
>
> I think I wrote the initial classes, and I picked the values from a
> domain-specific case. So from a general point of view, these values are
> probably worthless.
>
> OK for deprecating in 3.1 and suppressing the values in 4.0.
>
I'm also +1 for this change.
Sébastien


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

Re: [Math] HarmonicFitter and GaussianFitter

2012-06-09 Thread Sébastien Brisard

2012/6/9 Luc Maisonobe :
> Le 09/06/2012 02:48, Gilles Sadowski a écrit :
>> On Sat, Jun 09, 2012 at 02:00:42AM +0200, Gilles Sadowski wrote:
>>> Hi.
>>>
>>> I'm a bit annoyed that "HarmonicFitter" and "GaussianFitter" suffer from the
>>> same design flaw as "PolynomialFitter", namely that it is possible to call
>>> the non-overridden "fit" with an argument that is not of the right type
>>> (i.e. "HarmonicOscillator.Parametric" and "Gaussian.Parametric",
>>> respectively).
>>> However we cannot simply remove them as in MATH-800, since those classes
>>> perform non-trivial computations.
>>> One possibility is to override the "fit" methods that should not be used,
>>> and have them throw an exception.
>>>
>>> What do you think?
>>> Which exception would be most appropriate?
>>
>> This doesn't work (I should have tested it before sending the message...).
>>
>> However, I think that using generics might solve the problem:
>> ---
>> public class CurveFitter
>> ---
>>
>> And thus
>> ---
>> public class GaussianFitter extends CurveFitter
>> ---
>>
>> Then, we could keep "PolynomialFitter".
>>
>>
>> OK for this change?
>
> OK.
>
> Luc
>
Sounds good to me!
Sébastien


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

[math] bugs in OpenMapRealVector

2012-06-09 Thread Sébastien Brisard

Hello,
while working on MATH-795, I've realized that OpenMapRealVector has
several bugs. This is reported in MATH-803. I need some help to find
an efficient solution for this bug!

Thanks in advance!
Sébastien


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-11 Thread Sébastien Brisard

Hi Gilles,

2012/6/11 Gilles Sadowski :
> Hello.
>
>> Date: Mon Jun 11 05:52:16 2012
>> New Revision: 1348721
>>
>> URL: http://svn.apache.org/viewvc?rev=1348721&view=rev
>> Log:
>> MATH-803:
>>   - modified OpenMapRealVector.ebeMultiply() and ebeDivide() to handle
>> special cases  0d * NaN, 0d * Infinity, 0d / 0d and 0d / NaN.
>>   - added implementation of isNaN() and isInfinite() to
>> SparseRealVectorTest.SparseRealVectorTestImpl in order to allow for testing
>> of OpenMapRealVector.ebeMultiply() and ebeDivide() with mixed types.
>>
>> Modified:
>>     
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
>>     
>> commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
>>
>> Modified: 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
>> URL: 
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java?rev=1348721&r1=1348720&r2=1348721&view=diff
>> ==
>> --- 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
>>  (original)
>> +++ 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
>>  Mon Jun 11 05:52:16 2012
>> @@ -341,10 +341,14 @@ public class OpenMapRealVector extends S
>>      public OpenMapRealVector ebeDivide(RealVector v) {
>>          checkVectorDimensions(v.getDimension());
>>          OpenMapRealVector res = new OpenMapRealVector(this);
>> -        Iterator iter = entries.iterator();
>> -        while (iter.hasNext()) {
>> -            iter.advance();
>> -            res.setEntry(iter.key(), iter.value() / v.getEntry(iter.key()));
>> +        /*
>> +         * MATH-803: it is not sufficient to loop through non zero entries 
>> of
>> +         * this only. Indeed, if this[i] = 0d and v[i] = 0d, then
>> +         * this[i] / v[i] = NaN, and not 0d.
>> +         */
>> +        final int n = getDimension();
>> +        for (int i = 0; i < n; i++) {
>> +            res.setEntry(i, this.getEntry(i) / v.getEntry(i));
>>          }
>>          return res;
>>      }
>
> I think that this renders the class potentially useless, if we assume that
> it is used when "large" vectors are needed.
> Indeed, after this operation, all the entries will be stored; thus
> cancelling the memory efficiency of the class, and probably making the
> returned value slower than an "ArrayRealVector" instance for subsequent
> operations.
>
I'm not sure that all entries would be stored (except if setEntry does
not distinguish between zero values and non-zero values).

> For every method that a "RealVector" argument, there should be a specialized
> implementation that take an "OpenMapRealVector".
>
> Also, couldn't we solve some of these problems if the value of the default
> entry was stored and mutable? E.g. if the default for "v" and "w" is zero,
> then the default for "v / w" will be "NaN".
>
>> @@ -359,6 +363,25 @@ public class OpenMapRealVector extends S
>>              iter.advance();
>>              res.setEntry(iter.key(), iter.value() * v.getEntry(iter.key()));
>>          }
>> +        /*
>> +         * MATH-803: the above loop assumes that 0d * x  = 0d for any 
>> double x,
>> +         * which allows to consider only the non-zero entries of this. 
>> However,
>> +         * this fails if this[i] == 0d and (v[i] = NaN or v[i] = Infinity).
>> +         *
>> +         * These special cases are handled below.
>> +         */
>> +        if (v.isNaN() || v.isInfinite()) {
>> +            final int n = getDimension();
>> +            for (int i = 0; i < n; i++) {
>> +                final double y = v.getEntry(i);
>> +                if (Double.isNaN(y)) {
>> +                    res.setEntry(i, Double.NaN);
>> +                } else if (Double.isInfinite(y)) {
>> +                    final double x = this.getEntry(i);
>> +                    res.setEntry(i, x * y);
>> +                }
>> +            }
>> +        }
>
> This probably can only be a temporary solution: 3 additional loops over all
> the elements...
>
If we cache isNaN and isInfinite, this is "only" one additional loop ;).
I don't like this solution either, but the bugs I have identified are
very real, except if we accept that exceptional cases are not handled
correctly by sparse vectors. I'm personally adverse to this option.
If you have a better idea, please feel free to change that. By the
way, would you like me to revert this commit ?

I just would like to mention that sparse vectors allow a gain in
memory. Speed is, in my view, a bonus, but you probably think the
opposite...

Amitiés,
Sébastien


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-11 Thread Sébastien Brisard

Hello Gilles,

2012/6/11 Gilles Sadowski :
> Hello Sébastien.
>
>> >> +        /*
>> >> +         * MATH-803: it is not sufficient to loop through non zero 
>> >> entries of
>> >> +         * this only. Indeed, if this[i] = 0d and v[i] = 0d, then
>> >> +         * this[i] / v[i] = NaN, and not 0d.
>> >> +         */
>> >> +        final int n = getDimension();
>> >> +        for (int i = 0; i < n; i++) {
>> >> +            res.setEntry(i, this.getEntry(i) / v.getEntry(i));
>> >>          }
>> >>          return res;
>> >>      }
>> >
>> > I think that this renders the class potentially useless, if we assume that
>> > it is used when "large" vectors are needed.
>> > Indeed, after this operation, all the entries will be stored; thus
>> > cancelling the memory efficiency of the class, and probably making the
>> > returned value slower than an "ArrayRealVector" instance for subsequent
>> > operations.
>> >
>> I'm not sure that all entries would be stored (except if setEntry does
>> not distinguish between zero values and non-zero values).
>
> The problem is when the common values are not the default one, like ...
>
>>
>> > For every method that a "RealVector" argument, there should be a 
>> > specialized
>> > implementation that take an "OpenMapRealVector".
>> >
>> > Also, couldn't we solve some of these problems if the value of the default
>> > entry was stored and mutable? E.g. if the default for "v" and "w" is zero,
>> > then the default for "v / w" will be "NaN".
>
> ... in this example.
>
Ooops, missed this one!
Yes that was an improvement I was thinking of (in fact, the unit tests
are already implemented with this idea in mind).
 This would allow efficient implementation of the cosine for example.
I didn't think about the application you suggest, but I like this idea
very much.

How about we leave it as is for the time being, and we open a JIRA
ticket for this improvement, explicitely mentioning that ebeDivide()
and ebeMultiply() should be revisited in order to improve their
efficiency?

This does not solve the whole issue, however, because if the default
entry is zero, its sign is lost, and {finite value} / {zero} is of
undetermined sign. Any idea regarding this point?

Sébastien


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-11 Thread Sébastien Brisard

Hello,

2012/6/11 Gilles Sadowski :
> Hi.
>
>> >> >> +         * MATH-803: it is not sufficient to loop through non zero 
>> >> >> entries of
>> >> >> +         * this only. Indeed, if this[i] = 0d and v[i] = 0d, then
>> >> >> +         * this[i] / v[i] = NaN, and not 0d.
>> >> >> +         */
>> >> >> +        final int n = getDimension();
>> >> >> +        for (int i = 0; i < n; i++) {
>> >> >> +            res.setEntry(i, this.getEntry(i) / v.getEntry(i));
>> >> >>          }
>> >> >>          return res;
>> >> >>      }
>> >> >
>> >> > I think that this renders the class potentially useless, if we assume 
>> >> > that
>> >> > it is used when "large" vectors are needed.
>> >> > Indeed, after this operation, all the entries will be stored; thus
>> >> > cancelling the memory efficiency of the class, and probably making the
>> >> > returned value slower than an "ArrayRealVector" instance for subsequent
>> >> > operations.
>> >> >
>> >> I'm not sure that all entries would be stored (except if setEntry does
>> >> not distinguish between zero values and non-zero values).
>> >
>> > The problem is when the common values are not the default one, like ...
>> >
>> >>
>> >> > For every method that a "RealVector" argument, there should be a 
>> >> > specialized
>> >> > implementation that take an "OpenMapRealVector".
>> >> >
>> >> > Also, couldn't we solve some of these problems if the value of the 
>> >> > default
>> >> > entry was stored and mutable? E.g. if the default for "v" and "w" is 
>> >> > zero,
>> >> > then the default for "v / w" will be "NaN".
>> >
>> > ... in this example.
>> >
>> Ooops, missed this one!
>> Yes that was an improvement I was thinking of (in fact, the unit tests
>> are already implemented with this idea in mind).
>>  This would allow efficient implementation of the cosine for example.
>> I didn't think about the application you suggest, but I like this idea
>> very much.
>>
>> How about we leave it as is for the time being, and we open a JIRA
>> ticket for this improvement, explicitely mentioning that ebeDivide()
>> and ebeMultiply() should be revisited in order to improve their
>> efficiency?
>
> I agree.
>
>>
>> This does not solve the whole issue, however, because if the default
>> entry is zero, its sign is lost, and {finite value} / {zero} is of
>> undetermined sign. Any idea regarding this point?
>
> I guess that we could also keep a flag for the sign.
>
Do you mean that the default value would be +0 or -0, in some zero
entries would be stored? Or do you mean to store the sign of each zero
entry,  which is slightly less efficient than the present
implementation, but would work OK? These signs could be stored in very
compact form (as bits), as they need only seldom be retrieved.

>
> Best regards,
> Gilles
>
> P.S. Maybe that we should also have a look at how other libraries handle
>     this kind of issues.
>
Thanks for the suggestion, I will have a look.

Best regards,
Sébastien


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-12 Thread Sébastien Brisard

2012/6/12 Gilles Sadowski :
> Hello.
>
>> > [...]
>> > I agree.
>> >
>> >>
>> >> This does not solve the whole issue, however, because if the default
>> >> entry is zero, its sign is lost, and {finite value} / {zero} is of
>> >> undetermined sign. Any idea regarding this point?
>> >
>> > I guess that we could also keep a flag for the sign.
>> >
>> Do you mean that the default value would be +0 or -0, in some zero
>> entries would be stored?
>
> I didn't get the end of the sentence.
>
Yes, something got lost in translation...
In your solution, some zero entries would be stored, while others
wouldn't. This is slightly inefficient memory-wise.

> Anyways, what I meant is indeed to be able to specify that the default zero
> is negative (so that "v / zero" has the correct sign).
>
>> Or do you mean to store the sign of each zero
>> entry,  which is slightly less efficient than the present
>> implementation, but would work OK?
>
> No.
>
>> These signs could be stored in very
>> compact form (as bits), as they need only seldom be retrieved.
>
> I strongly prefer a "boolean" unless you can devise a way to perform
> calculations that strongly benefit from bit manipulation.
>
> Anyways, let's not try to re-invent the wheel...
>
I fully agree. We could also opt for a less correct, but more
efficient solution: we do not store the sign of zero, and return NaN
each time v / zero occurs. The result should be NaN anyway, because
its sign is undecidable. This specificity would be clearly stated in
the javadoc. What do you think?

Sébastien


> Regards,
> Gilles
>
>> [...]
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-12 Thread Sébastien Brisard

Hello Gilles,

> [...]
>
>> I fully agree. We could also opt for a less correct, but more
>> efficient solution: we do not store the sign of zero, and return NaN
>> each time v / zero occurs. The result should be NaN anyway, because
>> its sign is undecidable. This specificity would be clearly stated in
>> the javadoc. What do you think?
>
> We had this sort of discussion with the "Complex" class, where there are
> such issues as "standard" vs "correct" vs "documented" etc. E.g. some
> computations return "NaN" where it should be "infinity" (or vice-versa, I
> don't remember exactly). See MATH-667:
>  https://issues.apache.org/jira/browse/MATH-667
>
> Maybe that we must be guided with some use-cases for this class.
> Maybe in actual uses, the problems discussed here do not appear, and any
> additional check would destroy the usefulness of this class in such cases
> (just a wild guess). Maybe that we should drop support for sparse vectors!
>
I fully agree with you. Speaking quite honestly as a user of the
linear package, even if I know that IEEE floats handle infinite values
well, I tend to dish results that contain those values, and try to
cure the cause of these unwanted values. So, as a user, whether the
method returns NaN of infinity does not matter to me (as long as it
does not return zero --or any normal value-- quietly): in any case, I
would do a computation again, this time with "correct" values.

I also think that in this case, strict adherence to IEEE standards
(which were written for scalar values, not vectors) should not
compromise the performance of the class.

I would be more cautious with your suggestion about getting rid of
sparse vectors. I think it's nice to have them, even if the support is
limited (we should of course concentrate on array-based vectors
first). As long as we clearly document these limits, I'd like to keep
them. Besides, with the new set of abstract tests being constructed
for any vector class, it should be possible in the future to propose
and test a better implementation. The problem we have is clearly lack
of feedback on these features (remember the debates we had on the
"sparseIterators"-- not perfect, but experience only can tell how to
improve).

Best regards,

Sébastien


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-15 Thread Sébastien Brisard

Hi Gilles,

>
>>
>> > [...]
>> >
>> >> I fully agree. We could also opt for a less correct, but more
>> >> efficient solution: we do not store the sign of zero, and return NaN
>> >> each time v / zero occurs. The result should be NaN anyway, because
>> >> its sign is undecidable. This specificity would be clearly stated in
>> >> the javadoc. What do you think?
>> >
>> > We had this sort of discussion with the "Complex" class, where there are
>> > such issues as "standard" vs "correct" vs "documented" etc. E.g. some
>> > computations return "NaN" where it should be "infinity" (or vice-versa, I
>> > don't remember exactly). See MATH-667:
>> >  https://issues.apache.org/jira/browse/MATH-667
>> >
>> > Maybe that we must be guided with some use-cases for this class.
>> > Maybe in actual uses, the problems discussed here do not appear, and any
>> > additional check would destroy the usefulness of this class in such cases
>> > (just a wild guess). Maybe that we should drop support for sparse vectors!
>> >
>> I fully agree with you. Speaking quite honestly as a user of the
>> linear package, even if I know that IEEE floats handle infinite values
>> well, I tend to dish results that contain those values, and try to
>> cure the cause of these unwanted values. So, as a user, whether the
>> method returns NaN of infinity does not matter to me (as long as it
>> does not return zero --or any normal value-- quietly): in any case, I
>> would do a computation again, this time with "correct" values.
>>
>> I also think that in this case, strict adherence to IEEE standards
>> (which were written for scalar values, not vectors) should not
>> compromise the performance of the class.
>
> Am I mistaken, or is this at odds with a fix that will make the "division"
> test pass? [I.e. If there is no way to distinguish between +0 and -0 as the
> default entry then "1 / default" will always be "+inf".]
>

Do you mean I am bending the rules in favour of this specific case? In
fact, it could be argued that the returned value *should* be NaN,
since its sign is not decidable. Isn't that the reason why NaNs were
created in the first place? If we clearly state that for sparse
vectors with zero default value, the sign is lost, then the logical
conclusion is that ebeDivide should return NaN instead of infinity.
This is only my point of view, and I'm quite happy with any decision
(since none is going to be perfect...).

>>
>> I would be more cautious with your suggestion about getting rid of
>> sparse vectors. I think it's nice to have them, even if the support is
>> limited (we should of course concentrate on array-based vectors
>> first). As long as we clearly document these limits, I'd like to keep
>> them. Besides, with the new set of abstract tests being constructed
>> for any vector class, it should be possible in the future to propose
>> and test a better implementation. The problem we have is clearly lack
>> of feedback on these features (remember the debates we had on the
>> "sparseIterators"-- not perfect, but experience only can tell how to
>> improve).
>
> Well, if we know that it doesn't work well and we don't know how to fix it
> while at the same time keep its usefulness (efficiency-wise), we have to
> raise the issue of the necessity to keep supporting it even though it seems
> that nobody uses it.
>
> For the time being (staying compatible), we should at least fix the failing
> unit test problem; hence decide for example (as you said before) that the
> purpose is to save memory, but the trade-off is loss of performance.
>
Yes, but the problem is that the inefficient fix has already been
implemented... And it does not fix all bugs.
Anyway, we could probably aim at the most correct solution (still to
be defined!), at the expense of performance, since a more efficient
implementation can easily be implemented through the newly implemented
visitor pattern.

> Maybe that we could also impose that it is forbidden to divide by a sparse
> vector whose default value is zero. This will avoid a check on all entries
> (at the expense of forbidding legitimate divisions, when all the entries
> have a non-default value; but then one would wonder why using a sparse
> vector...).
>
That's also an idea to explore. I have to say I still do not see
clearly what we should do. I quickly looked at BLAS, they apparently
do not provide an element-by-element division. I should look at
octave, scilab, numpy...

Sébastien
>
> Best,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-16 Thread Sébastien Brisard

Hi Gilles,

2012/6/16 Gilles Sadowski :
> Hi.
>
>> >>
>> >> > [...]
>> >> >
>> >> >> I fully agree. We could also opt for a less correct, but more
>> >> >> efficient solution: we do not store the sign of zero, and return NaN
>> >> >> each time v / zero occurs. The result should be NaN anyway, because
>> >> >> its sign is undecidable. This specificity would be clearly stated in
>> >> >> the javadoc. What do you think?
>> >> >
>> >> > We had this sort of discussion with the "Complex" class, where there are
>> >> > such issues as "standard" vs "correct" vs "documented" etc. E.g. some
>> >> > computations return "NaN" where it should be "infinity" (or vice-versa, 
>> >> > I
>> >> > don't remember exactly). See MATH-667:
>> >> >  https://issues.apache.org/jira/browse/MATH-667
>> >> >
>> >> > Maybe that we must be guided with some use-cases for this class.
>> >> > Maybe in actual uses, the problems discussed here do not appear, and any
>> >> > additional check would destroy the usefulness of this class in such 
>> >> > cases
>> >> > (just a wild guess). Maybe that we should drop support for sparse 
>> >> > vectors!
>> >> >
>> >> I fully agree with you. Speaking quite honestly as a user of the
>> >> linear package, even if I know that IEEE floats handle infinite values
>> >> well, I tend to dish results that contain those values, and try to
>> >> cure the cause of these unwanted values. So, as a user, whether the
>> >> method returns NaN of infinity does not matter to me (as long as it
>> >> does not return zero --or any normal value-- quietly): in any case, I
>> >> would do a computation again, this time with "correct" values.
>> >>
>> >> I also think that in this case, strict adherence to IEEE standards
>> >> (which were written for scalar values, not vectors) should not
>> >> compromise the performance of the class.
>> >
>> > Am I mistaken, or is this at odds with a fix that will make the "division"
>> > test pass? [I.e. If there is no way to distinguish between +0 and -0 as the
>> > default entry then "1 / default" will always be "+inf".]
>> >
>>
>> Do you mean I am bending the rules in favour of this specific case?
>
> I'm just referring to that failure:
> ---
> java.lang.AssertionError: entry #14, left = 1.0, right = -0.0 
> expected:<-Infinity> but was:
> ---
>
> I.e. you can never make this test pass if you do not keep the sign.
>
Agreed

>
>> In
>> fact, it could be argued that the returned value *should* be NaN,
>> since its sign is not decidable. Isn't that the reason why NaNs were
>> created in the first place? If we clearly state that for sparse
>> vectors with zero default value, the sign is lost, then the logical
>> conclusion is that ebeDivide should return NaN instead of infinity.
>> This is only my point of view, and I'm quite happy with any decision
>> (since none is going to be perfect...).
>
> I'm not convinced; I'd rather throw an exception.
>
OK, that's an option.

>> >>
>> >> I would be more cautious with your suggestion about getting rid of
>> >> sparse vectors. I think it's nice to have them, even if the support is
>> >> limited (we should of course concentrate on array-based vectors
>> >> first). As long as we clearly document these limits, I'd like to keep
>> >> them. Besides, with the new set of abstract tests being constructed
>> >> for any vector class, it should be possible in the future to propose
>> >> and test a better implementation. The problem we have is clearly lack
>> >> of feedback on these features (remember the debates we had on the
>> >> "sparseIterators"-- not perfect, but experience only can tell how to
>> >> improve).
>> >
>> > Well, if we know that it doesn't work well and we don't know how to fix it
>> > while at the same time keep its usefulness (efficiency-wise), we have to
>> > raise the issue of the necessity to keep supporting it even though it seems
>> > that nobody uses it.
>> >
>> > For the time being (staying compatible), we should at least fix the failing
>> > unit test problem; hence decide for example (as you said before) that the
>> > purpose is to save memory, but the trade-off is loss of performance.
>> >
>> Yes, but the problem is that the inefficient fix has already been
>> implemented... And it does not fix all bugs.
>> Anyway, we could probably aim at the most correct solution (still to
>> be defined!), at the expense of performance, since a more efficient
>> implementation can easily be implemented through the newly implemented
>> visitor pattern.
>
>
> If the visitor can compute a correct answer more efficiently, then IMO we
> should readily deprecate the method and remove it ASAP.
>
Sorry, that wasn't what I meant. I meant that a visitor can compute a
less correct answer more efficiently. So if users can cope with
special values incorrectly handled, this opportunity is offered to
them, while the default implementation is more correct.

>>
>> > Maybe that we could also impose that it is forbidden to divide by a sparse
>> > vector whose default

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-16 Thread Sébastien Brisard

Hi Gilles,
and thanks for your time spent on this issue!

2012/6/16 Gilles Sadowski :
>> [...]
>>
>> >>
>> >> > Maybe that we could also impose that it is forbidden to divide by a 
>> >> > sparse
>> >> > vector whose default value is zero. This will avoid a check on all 
>> >> > entries
>> >> > (at the expense of forbidding legitimate divisions, when all the entries
>> >> > have a non-default value; but then one would wonder why using a sparse
>> >> > vector...).
>> >
>> > [...]
>>
>> Gilles, I have the feeling that your prefered option is to store the
>> sign of the default value.
>
> Not really; if there are no obvious drawbacks, I'd prefer the above option.
> I.e. something like
>
>  public OpenMapRealVector ebeDivide(OpenMapRealVector v) {
>    if (v.getDefaultEntry() == 0) {
>      throw new ZeroException();
>    }
>
>    // ...
>  }
>
>> In theory, that would solve the problem. I
>> see two objections
>> 1. sparse vectors would be less sparse (most users do not care about
>> the sign of the zero entries!)
>
> I agree; division by zero is in most (all?) cases a bug. So thoses users
> won't care about the above solution (or will be happy that their code fails
> early instead of propagating NaNs).
>
Yes, I think it's a good point (I would be glad if *any* calculation
could be stored when NaNs occured...).
What I can do is add some details in the javadoc, stating that we
opted for the exception option for robustness reasons, which leads to
less efficient code. I can suggest to use visitors if a more efficient
(and fragile !) solution is seeked.


>> 2. there is a threshold to decided whether or not an entry is actually
>> zero. So the current implementation already assumes the sign is lost.
>
> That's why the unit test fails. I think that it is more robust to make it
> explicit (by throwing an exception) that division by zero is not supported.
>
>> I'm really confused about the best solution to adopt.
>
> So am I.
> But my preference is to be more robust even if less efficient and not
> supporting corner cases.
>
I will implement the exception option, and alter the unit tests accordingly.

Best regards, and thanks again for your help on this issue. That was
an interesting discussion!

Sébastien


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

Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

2012-06-17 Thread Sébastien Brisard

Hello,

2012/6/16 Sébastien Brisard :
> Hi Gilles,
> and thanks for your time spent on this issue!
>
> 2012/6/16 Gilles Sadowski :
>>> [...]
>>>
>>> >>
>>> >> > Maybe that we could also impose that it is forbidden to divide by a 
>>> >> > sparse
>>> >> > vector whose default value is zero. This will avoid a check on all 
>>> >> > entries
>>> >> > (at the expense of forbidding legitimate divisions, when all the 
>>> >> > entries
>>> >> > have a non-default value; but then one would wonder why using a sparse
>>> >> > vector...).
>>> >
>>> > [...]
>>>
>>> Gilles, I have the feeling that your prefered option is to store the
>>> sign of the default value.
>>
>> Not really; if there are no obvious drawbacks, I'd prefer the above option.
>> I.e. something like
>>
>>  public OpenMapRealVector ebeDivide(OpenMapRealVector v) {
>>    if (v.getDefaultEntry() == 0) {
>>      throw new ZeroException();
>>    }
>>
>>    // ...
>>  }
>>
>>> In theory, that would solve the problem. I
>>> see two objections
>>> 1. sparse vectors would be less sparse (most users do not care about
>>> the sign of the zero entries!)
>>
>> I agree; division by zero is in most (all?) cases a bug. So thoses users
>> won't care about the above solution (or will be happy that their code fails
>> early instead of propagating NaNs).
>>
> Yes, I think it's a good point (I would be glad if *any* calculation
> could be stored when NaNs occured...).
> What I can do is add some details in the javadoc, stating that we
> opted for the exception option for robustness reasons, which leads to
> less efficient code. I can suggest to use visitors if a more efficient
> (and fragile !) solution is seeked.
>
>
>>> 2. there is a threshold to decided whether or not an entry is actually
>>> zero. So the current implementation already assumes the sign is lost.
>>
>> That's why the unit test fails. I think that it is more robust to make it
>> explicit (by throwing an exception) that division by zero is not supported.
>>
>>> I'm really confused about the best solution to adopt.
>>
>> So am I.
>> But my preference is to be more robust even if less efficient and not
>> supporting corner cases.
>>
> I will implement the exception option, and alter the unit tests accordingly.
>
> Best regards, and thanks again for your help on this issue. That was
> an interesting discussion!
>
> Sébastien

well, I was hoping that we could close this issue, but I have realized
that the problem is more wide-spread. Because the subject of this
thread is not very explicit, I'm afraid not everyone is aware of these
discussions. I'll create a new thread.

Amitiés,
Sébastien


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

[math] Problems with sparse implementations of RealVector

2012-06-17 Thread Sébastien Brisard

Dear all,

in this thread,
http://markmail.org/thread/hhvm6wv3d3uhkwqs
we had an interesting discussion on a bug which was revealed by
abstract unit tests on all implementations of RealVector. It turns out
that the bug is more far-reaching than we initially thought, and I
would like to make sure that it has been brought to everyone's
attention (as the subject of the previous thread was pretty cryptic).

So here goes. In RealVector, we provide ebeMultiply(RealVector) and
ebeDivide(RealVector). Also, in sparse implementations of RealVector,
zero entries are not stored. This is all very well, but for the fact
that 0.0 is actually signed in Java. The sign of zero is effectively
lost in OpenMapRealVector. This affects the correctness of the
returned values of ebeMulltiply() and ebeDivide()

1. For ebeMultiply()
final RealVector v1 = new ArrayRealVector(new double[] { 1d });
final RealVector v2 = new OpenMapRealVector(new double[] { -0d });
final RealVector w = v1.ebeMultiply(v2);
System.out.println(1d / w.getEntry(0));

prints Infinity, instead of -Infinity (because the sign is lost in
v2). This means that w holds +0d instead of -0d.

2. For ebeDivide()
final RealVector v1 = new ArrayRealVector(new double[] { 1d });
final RealVector v2 = new OpenMapRealVector(new double[] { -0d });
final RealVector w = v1.ebeDivide(v2);
System.out.println(w.getEntry(0));

prints Infinity, instead of -Infinity. For this last bug, Gilles
suggested the following fix

>
>  public OpenMapRealVector ebeDivide(OpenMapRealVector v) {
>if (v.getDefaultEntry() == 0) {
>  throw new ZeroException();
>}
>
>// ...
>  }
>

which was indeed no big deal, since the exception occured only when
the expected entry should have been + or -Infinity (which means that
the calculation had effectively failed).

However, this fix is not the end of the story, because it should be
applied to *any* implementation of RealVector.ebeDivide, as long as
the provided argument is an OpenMapRealVector. This makes things
cumbersome. Also, other implementations of RealVector (not only
OpenMapRealVector) might be affected by the same limitation. In my
view, this would require the definition of a new abstract method in
RealVector
protected boolean preservesSignOfZeroEntries()
which returns true if the sign of zero entries can be reliably
retrieve from this vector. Then, for each implementation of
ebeMultiply and ebeDivide,, we should test for
preservesSignOfZeroEntries(), and handle the boundary cases
accordingly.

The question is then: how should the boundary case be handled in the
ebeMultiply example? In this case, the expected value is perfectly
valid, and throwing an exception would effectively stop a computation
which is not yet in failed state.  I would be tempted to quietly
accept operations like : any double * (zero with undecidable sign).
The returned value would be zero with undecidable sign (remember that
the sign of zero is only used to compute (any double) / (signed
zero)). But then, preservesSignOfZeroEntries() must be specified at
construction time, because even ArrayRealVector might in some
circumstances end up with zero entries with undecidable sign... This
quickly gets very complicated!

I think there is no satisfactory implementation of ebeMultiply and
ebeDivide, and I would go as far as deprecate them. Users who need to
perform these operations can always use visitors to do so efficiently
(if not in an absolute fool-proof way).

Any better idea?
Thanks in advance,
Sébastien


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

Re: [math] Problems with sparse implementations of RealVector

2012-06-19 Thread Sébastien Brisard

Hi Bill,

2012/6/19 Bill Barker :
>
>
> -Original Message- From: Luc Maisonobe
> Sent: Monday, June 18, 2012 1:40 AM
> To: Commons Developers List
> Subject: Re: [math] Problems with sparse implementations of RealVector
>
>
>> Hi Sébastien,
>>
>> Le 18/06/2012 08:11, Sébastien Brisard a écrit :
>>>
>>> Dear all,
>>>
>>> in this thread,
>>> http://markmail.org/thread/hhvm6wv3d3uhkwqs
>>> we had an interesting discussion on a bug which was revealed by
>>> abstract unit tests on all implementations of RealVector. It turns out
>>> that the bug is more far-reaching than we initially thought, and I
>>> would like to make sure that it has been brought to everyone's
>>> attention (as the subject of the previous thread was pretty cryptic).
>>>
>>> So here goes. In RealVector, we provide ebeMultiply(RealVector) and
>>> ebeDivide(RealVector). Also, in sparse implementations of RealVector,
>>> zero entries are not stored. This is all very well, but for the fact
>>> that 0.0 is actually signed in Java. The sign of zero is effectively
>>> lost in OpenMapRealVector. This affects the correctness of the
>>> returned values of ebeMulltiply() and ebeDivide()
>>>
>>> 1. For ebeMultiply()
>>>        final RealVector v1 = new ArrayRealVector(new double[] { 1d });
>>>        final RealVector v2 = new OpenMapRealVector(new double[] { -0d });
>>>        final RealVector w = v1.ebeMultiply(v2);
>>>        System.out.println(1d / w.getEntry(0));
>>>
>>> prints Infinity, instead of -Infinity (because the sign is lost in
>>> v2). This means that w holds +0d instead of -0d.
>>>
>>> 2. For ebeDivide()
>>>        final RealVector v1 = new ArrayRealVector(new double[] { 1d });
>>>        final RealVector v2 = new OpenMapRealVector(new double[] { -0d });
>>>        final RealVector w = v1.ebeDivide(v2);
>>>        System.out.println(w.getEntry(0));
>>>
>>> prints Infinity, instead of -Infinity. For this last bug, Gilles
>>> suggested the following fix
>>>
>>>>
>>>>  public OpenMapRealVector ebeDivide(OpenMapRealVector v) {
>>>>   if (v.getDefaultEntry() == 0) {
>>>>     throw new ZeroException();
>>>>   }
>>>>
>>>>   // ...
>>>>  }
>>>>
>>>
>>> which was indeed no big deal, since the exception occured only when
>>> the expected entry should have been + or -Infinity (which means that
>>> the calculation had effectively failed).
>>>
>>> However, this fix is not the end of the story, because it should be
>>> applied to *any* implementation of RealVector.ebeDivide, as long as
>>> the provided argument is an OpenMapRealVector. This makes things
>>> cumbersome. Also, other implementations of RealVector (not only
>>> OpenMapRealVector) might be affected by the same limitation. In my
>>> view, this would require the definition of a new abstract method in
>>> RealVector
>>> protected boolean preservesSignOfZeroEntries()
>>> which returns true if the sign of zero entries can be reliably
>>> retrieve from this vector. Then, for each implementation of
>>> ebeMultiply and ebeDivide,, we should test for
>>> preservesSignOfZeroEntries(), and handle the boundary cases
>>> accordingly.
>>>
>>> The question is then: how should the boundary case be handled in the
>>> ebeMultiply example? In this case, the expected value is perfectly
>>> valid, and throwing an exception would effectively stop a computation
>>> which is not yet in failed state.  I would be tempted to quietly
>>> accept operations like : any double * (zero with undecidable sign).
>>> The returned value would be zero with undecidable sign (remember that
>>> the sign of zero is only used to compute (any double) / (signed
>>> zero)). But then, preservesSignOfZeroEntries() must be specified at
>>> construction time, because even ArrayRealVector might in some
>>> circumstances end up with zero entries with undecidable sign... This
>>> quickly gets very complicated!
>>>
>>> I think there is no satisfactory implementation of ebeMultiply and
>>> ebeDivide, and I would go as far as deprecate them. Users who need to
>>> perform these operations can always use visitors to do so efficiently
>>> (if not in an absolute fool-proof way).
>>
>>
>> This sound good to me. I am not a big fan of all the ebe methods
>> (despite I think I am the one who implemented them, from a user
>> request). I also would be glad if we removed most or even all of the map
>> methods.
>>
>
> The ebe methods aren't all that interesting, and with the new visitor
> pattern they can be implemented by the user.  Also, the users of
> SparseVector really won't care what value of +-infinity and/or NaN is stored
> and would probably just prefer that an exception is thrown if this case is
> detected.
>
I agree. I miss the good old "division by zero" error...
Sébastien


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

Re: [math] Problems with sparse implementations of RealVector

2012-06-19 Thread Sébastien Brisard

Hi Bill,

>
> And, it is much worse than that.  Pretty much nobody cares about ebe, but
> dotProduct and outerProduct also assume that 0*NaN = 0 and 0*+-Infinity = 0.
> e.g.:
>  RealVector a = new OpenMapRealVector(10);
>  RealVector b = new OpenMapRealVector(10);
>  a.setEntry(1, 1.0);
>  b.setEntry(2, Double.NaN);
>  double prod = a.dotProduct(b);
>  assert(prod == 0.0);
>
> The OpenMapRealVector class is already so incredibly slow.  I really can't
> see maintaining support for it if it has to handle these edge cases as well.
>

Thanks for spotting this. I am in the middle of refactoring all unit
tests for RealVector, and haven't reached dotProduct() yet, so haven't
had the opportunity to reveal this bug.
I think the question you raise is legitimate. Gilles already
questioned in a recent post the support for this class.

What do the others think?

Sébastien


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

Re: svn commit: r1353140 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java

2012-06-23 Thread Sébastien Brisard

Hi Gilles,


2012/6/23 Gilles Sadowski :
> Hello.
>
> On Sat, Jun 23, 2012 at 03:09:15PM -, celes...@apache.org wrote:
>> Author: celestin
>> Date: Sat Jun 23 15:09:14 2012
>> New Revision: 1353140
>
> Either I don't understand this change, or I don't agree with it.
>
>>
>> URL: http://svn.apache.org/viewvc?rev=1353140&view=rev
>> Log:
>> In o.a.c.m3.Incrementor, modified constructor to allow for null values of 
>> the MaxCountExceededCallback. Null value was previously not checked wich 
>> could lead to a NullPointerException much later (at exhaustion of the 
>> counter).
>>
>> Modified:
>>     
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java
>>
>> Modified: 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java
>> URL: 
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java?rev=1353140&r1=1353139&r2=1353140&view=diff
>> ==
>> --- 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java
>>  (original)
>> +++ 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java
>>  Sat Jun 23 15:09:14 2012
>> @@ -58,13 +58,7 @@ public class Incrementor {
>>       * @param max Maximal count.
>>       */
>>      public Incrementor(int max) {
>> -        this(max,
>> -             new MaxCountExceededCallback() {
>> -                 /** {@inheritDoc} */
>> -                 public void trigger(int max) {
>> -                     throw new MaxCountExceededException(max);
>> -                 }
>> -             });
>> +        this(max, null);
>>      }
>
> You set the callback to "null" here but...
>
>>      /**
>> @@ -72,12 +66,22 @@ public class Incrementor {
>>       * counter exhaustion.
>>       *
>>       * @param max Maximal count.
>> -     * @param cb Function to be called when the maximal count has been 
>> reached.
>> +     * @param cb Function to be called when the maximal count has been 
>> reached
>> +     * (can be {@code null}).
>>       */
>>      public Incrementor(int max,
>>                         MaxCountExceededCallback cb) {
>>          maximalCount = max;
>> -        maxCountCallback = cb;
>> +        if (cb != null) {
>> +            maxCountCallback = cb;
>> +        } else {
>> +            maxCountCallback = new MaxCountExceededCallback() {
>> +                /** {@inheritDoc} */
>> +                public void trigger(int max) {
>> +                    throw new MaxCountExceededException(max);
>> +                }
>> +            };
>> +        }
>>      }
>
> ... here, when it *is* null, you set it to something not null. So it seems
> to be a contorted way of disallowing null while saying the opposite in the
> Javadoc!
>
> The first constructor was fine as it was.
> In the second, a check for not null was missing:
> ---
>    public Incrementor(int max,
>                       MaxCountExceededCallback cb) {
>        if (cb == null) {
>            throw new NullArgumentException();
>        }
>        maximalCount = max;
>        maxCountCallback = cb;
>    }
> ---
>
> Best regards,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

Well, the idea is this: if cb is null, fall back to default instead of
throwing an exception, which I find quite unnecessary here. The first
constructor was indeed fine, but the new implementation of the second
constructor (including the test for null) would lead to duplicate
code. So the first constructor, with restricted number of parameters
(assuming default values for unspecified parameters) calls the second
constructor, which is the most general one. I should have thought that
avoiding code duplication in constructors was good practice.

>From your reaction, I assume you do not like the idea of null being a
meaningful value for the parameter cb? If so, I'm sorry. I really
thought that this change was rather minor. If you like, I can cancel
this change, and open a JIRA ticket.

Best regards,
Sébastien


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

Re: svn commit: r1353140 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java

2012-06-24 Thread Sébastien Brisard

Hi Gilles,

>
> Exactly: If a user does not care about the callback, he shouldn't even have
> to look at the second constructor, even less wonder about the consequence of
> setting it to null.
>
Well, it could also be argued that default parameters are *evil* (I do
think they are), and that explicitly setting a parameter to null
forces the user to think about his decision. However, our views are
more on the philosophical plane, and I think we could argue for ages
without coming to a conclusion... Therefore...

>
> No ticket is necessary (IMHO). I'm fine with just reverting, then adding the
> precondition block: it will clearly express that a callback *is* necessary.
> [Thanks for spotting that bug.]
>
... reverted to previous implementation of constructors (with
additional check for null) in r1353386. Thanks for reviewing this
commit which was not in line with the remainder of Commons-Math3.
Please note that at some point, I will have to take a look to
iterative linear solvers, where (as far as I remember) I allowed the
specification of a null preconditioner (sorry, sorry, sorry!).

Amitiés,
Sébastien


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

Re: svn commit: r1353386 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util: Incrementor.java IterationManager.java

2012-06-25 Thread Sébastien Brisard

Hello Gilles,

2012/6/25 Gilles Sadowski :
> Hello Sébastien.
>
>>      /**
>> @@ -66,22 +72,16 @@ public class Incrementor {
>>       * counter exhaustion.
>>       *
>>       * @param max Maximal count.
>> -     * @param cb Function to be called when the maximal count has been 
>> reached
>> -     * (can be {@code null}).
>> +     * @param cb Function to be called when the maximal count has been 
>> reached.
>> +     * @throws NullPointerException if {@code cb} is {@code null}
>                  
>
>>       */
>>      public Incrementor(int max,
>>                         MaxCountExceededCallback cb) {
>> -        maximalCount = max;
>> -        if (cb != null) {
>> -            maxCountCallback = cb;
>> -        } else {
>> -            maxCountCallback = new MaxCountExceededCallback() {
>> -                /** {@inheritDoc} */
>> -                public void trigger(int max) {
>> -                    throw new MaxCountExceededException(max);
>> -                }
>> -            };
>> +        if (cb == null){
>> +            throw new NullPointerException();
>
> See comment in class "o.a.c.m.NullArgumentException".
>
I'm sorry, I totally missed that (and you did use the correct
exception in your previous post...).

>>          }
>> +        maximalCount = max;
>> +        maxCountCallback = cb;
>>      }
>>
>>      /**
>>
>> Modified: 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java
>> URL: 
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java?rev=1353386&r1=1353385&r2=1353386&view=diff
>> ==
>> --- 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java
>>  (original)
>> +++ 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java
>>  Mon Jun 25 05:22:58 2012
>> @@ -43,7 +43,8 @@ public class IterationManager {
>>       * @param maxIterations the maximum number of iterations
>>       */
>>      public IterationManager(final int maxIterations) {
>> -        this(maxIterations, null);
>> +        this.iterations = new Incrementor(maxIterations);
>> +        this.listeners = new CopyOnWriteArrayList();
>>      }
>>
>>      /**
>> @@ -51,10 +52,14 @@ public class IterationManager {
>>       *
>>       * @param maxIterations the maximum number of iterations
>>       * @param callBack the function to be called when the maximum number of
>> -     * iterations has been reached (can be {@code null})
>> +     * iterations has been reached
>> +     * @throws NullPointerException if {@code callBack} is {@code null}
>                  
>
>>       */
>>      public IterationManager(final int maxIterations,
>>                              final Incrementor.MaxCountExceededCallback 
>> callBack) {
>> +        if (callBack == null) {
>> +            throw new NullPointerException();
>
> Ditto.
>
>> +        }
>>          this.iterations = new Incrementor(maxIterations, callBack);
>>          this.listeners = new CopyOnWriteArrayList();
>>      }
>>
>
> Best,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

Corrected in r1353451. Thanks again for reviewing this (and for your
patience...).
Sébastien


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

Re: svn commit: r1353386 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util: Incrementor.java IterationManager.java

2012-06-25 Thread Sébastien Brisard

Hi,
can't believe I didn't spot that earlier... Should be OK now (at
last!!!) -- see r1353586.
Best regards,
Sébastien

2012/6/25 Gilles Sadowski :
> Hi.
>
>> 2012/6/25 Gilles Sadowski :
>> > Hello Sébastien.
>> >
>> >>      /**
>> >> @@ -66,22 +72,16 @@ public class Incrementor {
>> >>       * counter exhaustion.
>> >>       *
>> >>       * @param max Maximal count.
>> >> -     * @param cb Function to be called when the maximal count has been 
>> >> reached
>> >> -     * (can be {@code null}).
>> >> +     * @param cb Function to be called when the maximal count has been 
>> >> reached.
>> >> +     * @throws NullPointerException if {@code cb} is {@code null}
>> >                  
>> >
>> >>       */
>> >>      public Incrementor(int max,
>> >>                         MaxCountExceededCallback cb) {
>> >> -        maximalCount = max;
>> >> -        if (cb != null) {
>> >> -            maxCountCallback = cb;
>> >> -        } else {
>> >> -            maxCountCallback = new MaxCountExceededCallback() {
>> >> -                /** {@inheritDoc} */
>> >> -                public void trigger(int max) {
>> >> -                    throw new MaxCountExceededException(max);
>> >> -                }
>> >> -            };
>> >> +        if (cb == null){
>> >> +            throw new NullPointerException();
>> >
>> > See comment in class "o.a.c.m.NullArgumentException".
>> >
>> I'm sorry, I totally missed that (and you did use the correct
>> exception in your previous post...).
>
> Thanks for the change.
>
>>
>> >>          }
>> >> +        maximalCount = max;
>> >> +        maxCountCallback = cb;
>> >>      }
>> >>
>> >>      /**
>> >>
>> >> Modified: 
>> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java
>> >> URL: 
>> >> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java?rev=1353386&r1=1353385&r2=1353386&view=diff
>> >> ==
>> >> --- 
>> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java
>> >>  (original)
>> >> +++ 
>> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/IterationManager.java
>> >>  Mon Jun 25 05:22:58 2012
>> >> @@ -43,7 +43,8 @@ public class IterationManager {
>> >>       * @param maxIterations the maximum number of iterations
>> >>       */
>> >>      public IterationManager(final int maxIterations) {
>> >> -        this(maxIterations, null);
>> >> +        this.iterations = new Incrementor(maxIterations);
>> >> +        this.listeners = new CopyOnWriteArrayList();
>> >>      }
>> >>
>> >>      /**
>> >> @@ -51,10 +52,14 @@ public class IterationManager {
>> >>       *
>> >>       * @param maxIterations the maximum number of iterations
>> >>       * @param callBack the function to be called when the maximum number 
>> >> of
>> >> -     * iterations has been reached (can be {@code null})
>> >> +     * iterations has been reached
>> >> +     * @throws NullPointerException if {@code callBack} is {@code null}
>> >                  
>> >
>> >>       */
>> >>      public IterationManager(final int maxIterations,
>> >>                              final Incrementor.MaxCountExceededCallback 
>> >> callBack) {
>> >> +        if (callBack == null) {
>> >> +            throw new NullPointerException();
>> >
>> > Ditto.
>
> Sorry I didn't noticed before but I think that this check is redundant
> with ...
>
>> >
>> >> +        }
>> >>          this.iterations = new Incrementor(maxIterations, callBack);
>
> ... the one now performed at Incrementor's construction.
>
>> >>          this.listeners = new CopyOnWriteArrayList();
>> >>      }
>> >>
>> >
>> > Best,
>> > Gilles
>> >
>> > -
>> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> > For additional commands, e-mail: dev-h...@commons.apache.org
>> >
>>
>> Corrected in r1353451. Thanks again for reviewing this (and for your
>> patience...).
>
>
> Thanks,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: [Math] Issue 810

2012-07-02 Thread Sébastien Brisard

Hi,

2012/7/2 Gilles Sadowski :
> Hi.
>
> Please have a look at
>   https://issues.apache.org/jira/browse/MATH-810
>
> [This will enable more natural usage in contexts where there is no notion of
> "key" and "value".]
>
> Any objection?
>
>
> Best,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

I'm +1 for this change. I would even write the patch the other way round

public K getKey() {
  return getFirst();
}

to emphasize that a Key, Value pair is just a particular case of a
general pair. This is a bit nit-picking, though...

Sébastien


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

Re: [Math] Issue 810

2012-07-02 Thread Sébastien Brisard

Hi,

2012/7/2 Gilles Sadowski :
> Hello.
>
>> >
>> > Please have a look at
>> >   https://issues.apache.org/jira/browse/MATH-810
>> >
>> > [This will enable more natural usage in contexts where there is no notion 
>> > of
>> > "key" and "value".]
>> >
>> > Any objection?
>> >
>> >
>> > Best,
>> > Gilles
>> >
>> > -
>> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> > For additional commands, e-mail: dev-h...@commons.apache.org
>> >
>>
>> I'm +1 for this change. I would even write the patch the other way round
>>
>> public K getKey() {
>>   return getFirst();
>> }
>>
>> to emphasize that a Key, Value pair is just a particular case of a
>> general pair. This is a bit nit-picking, though...
>
> While I agree on the principle, we could just leave it as is for "historical
> reasons". :-)
>

Yes of course, that was a joke.
Sébastien


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

[math] Fluent interface in RealVector

2012-07-07 Thread Sébastien Brisard

Hello,
most existing methods in class RealVector allow method chaining.
However, some methods just return void instead of this
  - addToEntry
  - set
  - setEntry
  - setSubVector
  - unitize

Are you OK with having all or only some (which ones) methods return this?

Best regards,
Sébastien


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

[math] On MATH-812

2012-07-07 Thread Sébastien Brisard

Hi,
MATH-812 is the reason why Cump keeps complaining these days. So
please have a look and let me know what option seems preferable.
Best regards,
Sébastien


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

[math] In class RealVector, sparseIterator() is confusing

2012-07-07 Thread Sébastien Brisard

Hi,
recent bugs have revealed that the use of sparseIterator() is quite
delicate. Also, the current Javadoc is confusing, since it says
"Specialized implementations may choose to not iterate over all
dimensions, either because those values are unset, or are equal to
defaultValue(), or are small enough to be ignored for the purposes of
iteration". This suggests that an unstored value different from zero
could be used (in fact, I have implemented RealVectorAbstractTest with
this goal in mind). However, method "defaultValue()" (mentioned in the
javadoc) does not exist, and when sparseIterators are invoked in the
current implementation of CM3, it is *always* assumed that this
default value is indeed zero.
At the very least, we need to alter the Javadoc.

I would personally be in favor of deprecating this method, at least in
the {{RealVector}} base class. I think it would make more sense to
move it to the {{SparseRealVector}} class. Also, I think that at some
point, we should have a discussion on the current implementation of
sparse vectors/matrices, since recent tests have shown flaws in the
implementation.

Looking forward to reading your thoughts on that topic,
Sébastien


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

Re: [math] Fluent interface in RealVector

2012-07-08 Thread Sébastien Brisard

2012/7/9 Gilles Sadowski :
> On Sat, Jul 07, 2012 at 01:49:25PM +0200, Sébastien Brisard wrote:
>> Hello,
>> most existing methods in class RealVector allow method chaining.
>
> Chaining does not always make for readable code.
>
>> However, some methods just return void instead of this
>>   - addToEntry
>>   - set
>>   - setEntry
>>   - setSubVector
>>   - unitize
>>
>> Are you OK with having all or only some (which ones) methods return this?
>
> +0 (for people who like it).
>
I agree, I generally find confusing methods which return {@code this},
but I have to admit that in this context, a fluent interface is very
useful.
Sébastien
>
> [And for API consistency.]
>
>
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>


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

Re: [math] Fluent interface in RealVector

2012-07-08 Thread Sébastien Brisard

Hello,

2012/7/9 Luc Maisonobe :
> On 09/07/2012 07:08, Sébastien Brisard wrote:
>> 2012/7/9 Gilles Sadowski :
>>> On Sat, Jul 07, 2012 at 01:49:25PM +0200, Sébastien Brisard wrote:
>>>> Hello,
>>>> most existing methods in class RealVector allow method chaining.
>>>
>>> Chaining does not always make for readable code.
>>>
>>>> However, some methods just return void instead of this
>>>>   - addToEntry
>>>>   - set
>>>>   - setEntry
>>>>   - setSubVector
>>>>   - unitize
>>>>
>>>> Are you OK with having all or only some (which ones) methods return this?
>>>
>>> +0 (for people who like it).
>>>
>> I agree, I generally find confusing methods which return {@code this},
>> but I have to admit that in this context, a fluent interface is very
>> useful.
>
> I think changing a return value from void to non-void is safe from a
> compatibility point of view, but I would like to be sure. Does anybody
> have an advice on it ?
>
That's what I would have thought, too. Would a silent clirr report be
convincing evidence (in which case, I'll try)?
We would also advertise this change in the release notes.
Sébastien


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

Re: [math] Fluent interface in RealVector

2012-07-08 Thread Sébastien Brisard

HI,

2012/7/9 Sébastien Brisard :
> Hello,
>
> 2012/7/9 Luc Maisonobe :
>> On 09/07/2012 07:08, Sébastien Brisard wrote:
>>> 2012/7/9 Gilles Sadowski :
>>>> On Sat, Jul 07, 2012 at 01:49:25PM +0200, Sébastien Brisard wrote:
>>>>> Hello,
>>>>> most existing methods in class RealVector allow method chaining.
>>>>
>>>> Chaining does not always make for readable code.
>>>>
>>>>> However, some methods just return void instead of this
>>>>>   - addToEntry
>>>>>   - set
>>>>>   - setEntry
>>>>>   - setSubVector
>>>>>   - unitize
>>>>>
>>>>> Are you OK with having all or only some (which ones) methods return this?
>>>>
>>>> +0 (for people who like it).
>>>>
>>> I agree, I generally find confusing methods which return {@code this},
>>> but I have to admit that in this context, a fluent interface is very
>>> useful.
>>
>> I think changing a return value from void to non-void is safe from a
>> compatibility point of view, but I would like to be sure. Does anybody
>> have an advice on it ?
>>
> That's what I would have thought, too. Would a silent clirr report be
> convincing evidence (in which case, I'll try)?
> We would also advertise this change in the release notes.
> Sébastien

As a follow-up: clirr does report an error. I'm not sure we should
treat it as such, though. I fail to see how this change can lead to
any binary compatibility issue. Will start a thread with a more
focused subject.
Sébastien


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

[all] Does this break binary compatibility?

2012-07-09 Thread Sébastien Brisard

All,
in Commons-Math, class RealVector has a method unitize() which divides
each component of this by the norm. The vector is changed in place.
The current signature of this method is
public void unitize()

Most methods in class RealVector implement a fluent API. I would like
unitize() to follow the same paradigm. In other words, I would like to
change the signature to
public RealVector unitize()
this method now returning this.

Question is: does this break binary compatibility? Although clirr
reports an error, I fail to see how this change can cause any problem.

Thanks in advance for your advice,
Sébastien


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

Re: [all] Does this break binary compatibility?

2012-07-09 Thread Sébastien Brisard

Hi,

>
> In fact, changing the return type does break binary compatibility (binary
> compatibility is different from source compatibility), since the return
> type of a method is part of is java signature.
> This means that if someone would want to use the new jar (with changed
> return type) as a drop-in replacement, without recompiling anything else,
> the code would not execute (without NoSuchMethodError thrown).
>
Thanks for these explanations. I guess this change will have to wait !

Sébastien


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

Re: [math] Fluent interface in RealVector

2012-07-09 Thread Sébastien Brisard

Hi,

2012/7/9 Sébastien Brisard :
> HI,
>
> 2012/7/9 Sébastien Brisard :
>> Hello,
>>
>> 2012/7/9 Luc Maisonobe :
>>> On 09/07/2012 07:08, Sébastien Brisard wrote:
>>>> 2012/7/9 Gilles Sadowski :
>>>>> On Sat, Jul 07, 2012 at 01:49:25PM +0200, Sébastien Brisard wrote:
>>>>>> Hello,
>>>>>> most existing methods in class RealVector allow method chaining.
>>>>>
>>>>> Chaining does not always make for readable code.
>>>>>
>>>>>> However, some methods just return void instead of this
>>>>>>   - addToEntry
>>>>>>   - set
>>>>>>   - setEntry
>>>>>>   - setSubVector
>>>>>>   - unitize
>>>>>>
>>>>>> Are you OK with having all or only some (which ones) methods return this?
>>>>>
>>>>> +0 (for people who like it).
>>>>>
>>>> I agree, I generally find confusing methods which return {@code this},
>>>> but I have to admit that in this context, a fluent interface is very
>>>> useful.
>>>
>>> I think changing a return value from void to non-void is safe from a
>>> compatibility point of view, but I would like to be sure. Does anybody
>>> have an advice on it ?
>>>
>> That's what I would have thought, too. Would a silent clirr report be
>> convincing evidence (in which case, I'll try)?
>> We would also advertise this change in the release notes.
>> Sébastien
>
> As a follow-up: clirr does report an error. I'm not sure we should
> treat it as such, though. I fail to see how this change can lead to
> any binary compatibility issue. Will start a thread with a more
> focused subject.
> Sébastien

It results from
http://mail-archives.apache.org/mod_mbox/commons-dev/201207.mbox/%3CCAGRH7HqdLSj65kBeD_tHfSA4%3DQh5BMPqqmNezxfZqZ5qjR%2Be6w%40mail.gmail.com%3E

that this change indeed breaks compatibility. I will therefore file a
JIRA ticket for 4.0...
Sébastien


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

[all] Is this disguised fishing?

2012-07-12 Thread Sébastien Brisard

Dear all,
please find below a message I've received. It was sent to my apache
address, which I barely use (but can easily be retrieved from my
apache login).
I'm just wondering whether this message might be a (very clever !)
attempt at fishing. I don't think so, it does not have the usual
flavour. Is it safe to answer? Has anyone else received such a
message?
Is there someone at Apache in charge of security which should be
informed of this message?

Thanks for your answers,
Sébastien


-- Forwarded message --
From: Yida Tao 
Date: 2012/7/12
Subject: Help developers understand program changes -- Seeking your feedback
To: celes...@apache.org


Dear Sébastien,

This is Yida Tao, a PhD student of Computer Science and Engineering in
Hong Kong University of Science and Technology. I’m currently working
on a research project using Apache Commons Math as an evaluation
subject.

Our observation is that program changes (or commits) sometimes mix
several issues together. For example, a single commit may actually fix
three different bugs, or it may perform some refactorings, fix a minor
bug and then implement a new feature. Such commit (we refer to it as
the “composite change”) might reduce the change’s understandability
and make the code review much harder.

Therefore, our research goal is to automatically decompose a composite
change into “slices”; each slice is aligned with one particular issue
addressed. For example, if a change fixes three different bugs, we
would like to see this change to be decomposed into three “slices”,
each corresponds to one bug-fix.

We would like to invite you to a short survey (around 5 - 10 minutes)
on our research result. In particular, below is an example of applying
our technique on one of your commits (revision 1210359 for Apache
Commons Math). Our technique automatically decomposes your commit into
3 slices. You can view all the slices of the commit at
http://webproject1.cse.ust.hk/ChangeSlicing/Survey/Math1210359.aspx

We really appreciate it if you could let us know your opinions on the
following two questions by replying this email:

1 Do you think the above example of slices is accurate?
(Please assign a value from 0 to 4:  0 - Not accurate at all, 1 - Not
accurate, 2 - Not sure, 3 - Accurate, 4 - Very accurate)

2 Do you think in general sliced changes (by our automatic change
slicing technique) would be useful for other developers to review and
understand your changes?
(Please assign a value from 0 to 4: 0 - Not useful at all, 1 - Not
useful, 2 - Not sure, 3 - Useful, 4 - Very useful)

Thank you for your time! Your feedback is very valuable to us!

Best Regards,
Yida Tao
PhD student
Department of Computer Science and Engineering
The Hong Kong University of Science and Technology
Email:  ida...@cse.ust.hk

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

Re: [all] Is this disguised fishing?

2012-07-12 Thread Sébastien Brisard

Hello,

>
> I have received messages like that in the past.  I just don't answer them.
>

That's quite wise indeed: no safety issue! On the other hand, if she
really is a PhD student, she might need a bit of feedback...
I like Adrian's answer: she could indeed post this message on the dev
ML, and would get much wider feedback on question #2.

Thanks for both taking the time to answer!
Sébastien


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

Re: [all] Is this disguised fishing?

2012-07-12 Thread Sébastien Brisard

Hi,
I have indeed redirected her to this mailing list. Let's see if she
does the move.
Best regards,
Sébastien


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

Re: svn commit: r1361164 - /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

Hi Gilles,

2012/7/13  :
> Author: erans
> Date: Fri Jul 13 11:20:42 2012
> New Revision: 1361164
>
> URL: http://svn.apache.org/viewvc?rev=1361164&view=rev
> Log:
> Two failing tests temporarily disabled until MATH-821 is fixed.
>
> Modified:
> 
> commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
>
> Modified: 
> commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
> URL: 
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java?rev=1361164&r1=1361163&r2=1361164&view=diff
> ==
> --- 
> commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
>  (original)
> +++ 
> commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
>  Fri Jul 13 11:20:42 2012
> @@ -97,4 +97,22 @@ public class SparseRealVectorTest extend
>  u.ebeMultiply(v1);
>  u.ebeDivide(v1);
>  }
> +
> +/**
> + * XXX This test is disabled because it currently fails.
> + * The bug must still be fixed in the sparse vector implementation.
> + * When this is done, this override should be deleted.
> + */
> +@Test
> +@Override
> +public void testMap() {}
> +
> +/**
> + * XXX This test is disabled because it currently fails.
> + * The bug must still be fixed in the sparse vector implementation.
> + * When this is done, this override should be deleted.
> + */
> +@Test
> +@Override
> +public void testMapToSelf() {}
>  }
>
>

We could simply have overriden createFunctions() in
RealVectorAbstractTest(), but your solution is fine! I just took the
liberty to add a @Ignore tag, which shows a message in the tests
reports, just in case we forget to reenable this test.

Best regards,
Sébastien


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

Re: [continuum] BUILD FAILURE: Apache Commons - Commons Math - Default Maven 2 Build Definition (Java 1.5)

Hi Gilles,

as you already mentioned, this bug is filed in MATH-821. I really need
some input, because I do not really know how to fix this bug in a
statisfactory way.

Best regards,
Sébastien


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

Re: svn commit: r1361164 - /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java

Hi,

2012/7/13 Gilles Sadowski :
> Hi.
>
>> [...] I just took the
>> liberty to add a @Ignore tag, which shows a message in the tests
>> reports, just in case we forget to reenable this test.
>
> Yes. That's cleaner. Thanks.
>
> However, the "skipped" message does not appear in the "txt" reports.
> It's there in the "xml" report. Is there a flag to set on the commad-line in
> order to generate HTML pages from those?
>
I think mvn site:site will generate a surefire report with those
messages included. As for the txt reports, I will investigate.

Best regards,
Sébastien


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

Re: svn commit: r1361164 - /commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java