You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Sébastien Brisard <se...@m4x.org> on 2011/09/08 08:07:56 UTC

[math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Hi,
this question was raised while working on MATH-659, which I quote
{quote}
In rev1166533, I removed the call to this method in
AdamsNordsieckTransformer.initializeHighOrderDerivatives(double,
double[], double[][], double[][]). This raises one important question.
All across the package o.a.c.m.ode, matrix variables are explicitely
declared as Array2DRowRealMatrix (as opposed to RealMatrix). This
leads to an ugly cast (see the end of the presently modified method).
I was wondering how necessary that was. Would it be thinkable to use
the most general supertype for all matrices?
{quote}
I should mention I have no knowledge at all about this package, so I
do apologize if it is naive.
Sébastien

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

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Sébastien Brisard <se...@m4x.org>.
I meant "compilation problem"...

Le 9 septembre 2011 09:37, Sébastien Brisard
<se...@m4x.org> a écrit :
> OK, thank you Luc. We are still doing some refactoring in the linear
> package. If this induces compilation of the ode package, I'll let you
> know.
> Sébastien
>
> 2011/9/9 Luc Maisonobe <Lu...@free.fr>:
>> Le 09/09/2011 04:05, Sébastien Brisard a écrit :
>>>
>>> Sorry Ted, I was not very clear in my explanations.
>>> solve does return a RealMatrix. Internally, it builds BlockRealMatrix,
>>> though.
>>> The issue is that Luc needs the underlying double[][] array in this
>>> ODE application.
>>
>> Yes, this is exactly the point.
>> In fact, this is also internal code user never sees. the matrices here
>> correspond to an internal state that is changed a very large number of time.
>>
>>> This you could easily get if the returned matrix were
>>> a Array2DRowRealMatrix, and you could have something along the lines
>>> {code}
>>> RealMatrix x = decomposition.getSolver().solve(new
>>> Array2DRowRealMatrix(b, false));
>>> if (x instanceof Array2DRowRealMatrix){
>>>     return x.getDataRef();
>>> } else {
>>>     return new Array2DRowRealMatrix(x.getData(), false);
>>> }
>>> {code}
>>> but this hack is of no use here, since it turns out that the
>>> decomposition solver being used here *always* returns a
>>> BlockRealMatrix (as an Array2DRealMatrix). So basically, deep copy of
>>> x through x.getData() will *always* occur. That's what is worrying me.
>>
>> Don't worry. As I wrote in the comment on this issue, this class is under
>> scrutiny for other changes due to other issues. I don't like either being
>> forced to use an Array2DRowRealMatrix. In fact I already tried to remove
>> them some months ago but failed, I had to go back to this signature. I will
>> make another attempt. As I may well change completely the way the internal
>> state is updated, I hope this will naturally simplify things.
>>
>> For now, just leave the multi step integrators classes alone (all classes
>> with Adams, Nordsieck and Multistep in their names, as well as BDF when it
>> will be committed later on), you would waste your time on them.
>>
>> Luc
>>
>>> I hope I do make my point, now.
>>> Sébastien
>>>
>>> 2011/9/9 Ted Dunning<te...@gmail.com>:
>>>>
>>>> Why doesn't solve just return a RealMatrix?  Why does it insist on
>>>> returning
>>>> an Array2DRowRealMatrix?
>>>>
>>>> Does the user really care?
>>>>
>>>> 2011/9/8 Sébastien Brisard<se...@m4x.org>
>>>>
>>>>> Hi Luc,
>>>>> thanks for your detailed explanations attached to the MATH-659. I'm
>>>>> worried about the changes I have applied to the code, now. Here is
>>>>> what I've done. I've replaced the following line
>>>>> {code}
>>>>> return new Array2DRowRealMatrix(decomposition.getSolver().solve(b),
>>>>> false);
>>>>> {code}
>>>>> with
>>>>> {code}
>>>>> RealMatrix x = decomposition.getSolver().solve(new
>>>>> Array2DRowRealMatrix(b, false));
>>>>> return new Array2DRowRealMatrix(x.getData(), false);
>>>>> {code}
>>>>>
>>>>> decomposition is in fact an instance of QRDecompositionImpl.Solver,
>>>>> whose method solve(RealMatrix) returns a BlockRealMatrix, not an
>>>>> Array2DRowRealMatrix, hence the ugly last line. This code seems to be
>>>>> correct (unit tests still pass), but I'm worried about the efficiency,
>>>>> especially if initializeHighOrderDerivatives is called very often.
>>>>> What do you think should be done?
>>>>>
>>>>> Best regards,
>>>>> Sébastien
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>

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

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Sébastien Brisard <se...@m4x.org>.
OK, thank you Luc. We are still doing some refactoring in the linear
package. If this induces compilation of the ode package, I'll let you
know.
Sébastien

2011/9/9 Luc Maisonobe <Lu...@free.fr>:
> Le 09/09/2011 04:05, Sébastien Brisard a écrit :
>>
>> Sorry Ted, I was not very clear in my explanations.
>> solve does return a RealMatrix. Internally, it builds BlockRealMatrix,
>> though.
>> The issue is that Luc needs the underlying double[][] array in this
>> ODE application.
>
> Yes, this is exactly the point.
> In fact, this is also internal code user never sees. the matrices here
> correspond to an internal state that is changed a very large number of time.
>
>> This you could easily get if the returned matrix were
>> a Array2DRowRealMatrix, and you could have something along the lines
>> {code}
>> RealMatrix x = decomposition.getSolver().solve(new
>> Array2DRowRealMatrix(b, false));
>> if (x instanceof Array2DRowRealMatrix){
>>     return x.getDataRef();
>> } else {
>>     return new Array2DRowRealMatrix(x.getData(), false);
>> }
>> {code}
>> but this hack is of no use here, since it turns out that the
>> decomposition solver being used here *always* returns a
>> BlockRealMatrix (as an Array2DRealMatrix). So basically, deep copy of
>> x through x.getData() will *always* occur. That's what is worrying me.
>
> Don't worry. As I wrote in the comment on this issue, this class is under
> scrutiny for other changes due to other issues. I don't like either being
> forced to use an Array2DRowRealMatrix. In fact I already tried to remove
> them some months ago but failed, I had to go back to this signature. I will
> make another attempt. As I may well change completely the way the internal
> state is updated, I hope this will naturally simplify things.
>
> For now, just leave the multi step integrators classes alone (all classes
> with Adams, Nordsieck and Multistep in their names, as well as BDF when it
> will be committed later on), you would waste your time on them.
>
> Luc
>
>> I hope I do make my point, now.
>> Sébastien
>>
>> 2011/9/9 Ted Dunning<te...@gmail.com>:
>>>
>>> Why doesn't solve just return a RealMatrix?  Why does it insist on
>>> returning
>>> an Array2DRowRealMatrix?
>>>
>>> Does the user really care?
>>>
>>> 2011/9/8 Sébastien Brisard<se...@m4x.org>
>>>
>>>> Hi Luc,
>>>> thanks for your detailed explanations attached to the MATH-659. I'm
>>>> worried about the changes I have applied to the code, now. Here is
>>>> what I've done. I've replaced the following line
>>>> {code}
>>>> return new Array2DRowRealMatrix(decomposition.getSolver().solve(b),
>>>> false);
>>>> {code}
>>>> with
>>>> {code}
>>>> RealMatrix x = decomposition.getSolver().solve(new
>>>> Array2DRowRealMatrix(b, false));
>>>> return new Array2DRowRealMatrix(x.getData(), false);
>>>> {code}
>>>>
>>>> decomposition is in fact an instance of QRDecompositionImpl.Solver,
>>>> whose method solve(RealMatrix) returns a BlockRealMatrix, not an
>>>> Array2DRowRealMatrix, hence the ugly last line. This code seems to be
>>>> correct (unit tests still pass), but I'm worried about the efficiency,
>>>> especially if initializeHighOrderDerivatives is called very often.
>>>> What do you think should be done?
>>>>
>>>> Best regards,
>>>> Sébastien
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

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

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 09/09/2011 04:05, Sébastien Brisard a écrit :
> Sorry Ted, I was not very clear in my explanations.
> solve does return a RealMatrix. Internally, it builds BlockRealMatrix, though.
> The issue is that Luc needs the underlying double[][] array in this
> ODE application.

Yes, this is exactly the point.
In fact, this is also internal code user never sees. the matrices here 
correspond to an internal state that is changed a very large number of time.

> This you could easily get if the returned matrix were
> a Array2DRowRealMatrix, and you could have something along the lines
> {code}
> RealMatrix x = decomposition.getSolver().solve(new
> Array2DRowRealMatrix(b, false));
> if (x instanceof Array2DRowRealMatrix){
>      return x.getDataRef();
> } else {
>      return new Array2DRowRealMatrix(x.getData(), false);
> }
> {code}
> but this hack is of no use here, since it turns out that the
> decomposition solver being used here *always* returns a
> BlockRealMatrix (as an Array2DRealMatrix). So basically, deep copy of
> x through x.getData() will *always* occur. That's what is worrying me.

Don't worry. As I wrote in the comment on this issue, this class is 
under scrutiny for other changes due to other issues. I don't like 
either being forced to use an Array2DRowRealMatrix. In fact I already 
tried to remove them some months ago but failed, I had to go back to 
this signature. I will make another attempt. As I may well change 
completely the way the internal state is updated, I hope this will 
naturally simplify things.

For now, just leave the multi step integrators classes alone (all 
classes with Adams, Nordsieck and Multistep in their names, as well as 
BDF when it will be committed later on), you would waste your time on them.

Luc

> I hope I do make my point, now.
> Sébastien
>
> 2011/9/9 Ted Dunning<te...@gmail.com>:
>> Why doesn't solve just return a RealMatrix?  Why does it insist on returning
>> an Array2DRowRealMatrix?
>>
>> Does the user really care?
>>
>> 2011/9/8 Sébastien Brisard<se...@m4x.org>
>>
>>> Hi Luc,
>>> thanks for your detailed explanations attached to the MATH-659. I'm
>>> worried about the changes I have applied to the code, now. Here is
>>> what I've done. I've replaced the following line
>>> {code}
>>> return new Array2DRowRealMatrix(decomposition.getSolver().solve(b), false);
>>> {code}
>>> with
>>> {code}
>>> RealMatrix x = decomposition.getSolver().solve(new
>>> Array2DRowRealMatrix(b, false));
>>> return new Array2DRowRealMatrix(x.getData(), false);
>>> {code}
>>>
>>> decomposition is in fact an instance of QRDecompositionImpl.Solver,
>>> whose method solve(RealMatrix) returns a BlockRealMatrix, not an
>>> Array2DRowRealMatrix, hence the ugly last line. This code seems to be
>>> correct (unit tests still pass), but I'm worried about the efficiency,
>>> especially if initializeHighOrderDerivatives is called very often.
>>> What do you think should be done?
>>>
>>> Best regards,
>>> Sébastien
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


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

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Sébastien Brisard <se...@m4x.org>.
Sorry Ted, I was not very clear in my explanations.
solve does return a RealMatrix. Internally, it builds BlockRealMatrix, though.
The issue is that Luc needs the underlying double[][] array in this
ODE application. This you could easily get if the returned matrix were
a Array2DRowRealMatrix, and you could have something along the lines
{code}
RealMatrix x = decomposition.getSolver().solve(new
Array2DRowRealMatrix(b, false));
if (x instanceof Array2DRowRealMatrix){
    return x.getDataRef();
} else {
    return new Array2DRowRealMatrix(x.getData(), false);
}
{code}
but this hack is of no use here, since it turns out that the
decomposition solver being used here *always* returns a
BlockRealMatrix (as an Array2DRealMatrix). So basically, deep copy of
x through x.getData() will *always* occur. That's what is worrying me.
I hope I do make my point, now.
Sébastien

2011/9/9 Ted Dunning <te...@gmail.com>:
> Why doesn't solve just return a RealMatrix?  Why does it insist on returning
> an Array2DRowRealMatrix?
>
> Does the user really care?
>
> 2011/9/8 Sébastien Brisard <se...@m4x.org>
>
>> Hi Luc,
>> thanks for your detailed explanations attached to the MATH-659. I'm
>> worried about the changes I have applied to the code, now. Here is
>> what I've done. I've replaced the following line
>> {code}
>> return new Array2DRowRealMatrix(decomposition.getSolver().solve(b), false);
>> {code}
>> with
>> {code}
>> RealMatrix x = decomposition.getSolver().solve(new
>> Array2DRowRealMatrix(b, false));
>> return new Array2DRowRealMatrix(x.getData(), false);
>> {code}
>>
>> decomposition is in fact an instance of QRDecompositionImpl.Solver,
>> whose method solve(RealMatrix) returns a BlockRealMatrix, not an
>> Array2DRowRealMatrix, hence the ugly last line. This code seems to be
>> correct (unit tests still pass), but I'm worried about the efficiency,
>> especially if initializeHighOrderDerivatives is called very often.
>> What do you think should be done?
>>
>> Best regards,
>> Sébastien
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>

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

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Ted Dunning <te...@gmail.com>.
Why doesn't solve just return a RealMatrix?  Why does it insist on returning
an Array2DRowRealMatrix?

Does the user really care?

2011/9/8 Sébastien Brisard <se...@m4x.org>

> Hi Luc,
> thanks for your detailed explanations attached to the MATH-659. I'm
> worried about the changes I have applied to the code, now. Here is
> what I've done. I've replaced the following line
> {code}
> return new Array2DRowRealMatrix(decomposition.getSolver().solve(b), false);
> {code}
> with
> {code}
> RealMatrix x = decomposition.getSolver().solve(new
> Array2DRowRealMatrix(b, false));
> return new Array2DRowRealMatrix(x.getData(), false);
> {code}
>
> decomposition is in fact an instance of QRDecompositionImpl.Solver,
> whose method solve(RealMatrix) returns a BlockRealMatrix, not an
> Array2DRowRealMatrix, hence the ugly last line. This code seems to be
> correct (unit tests still pass), but I'm worried about the efficiency,
> especially if initializeHighOrderDerivatives is called very often.
> What do you think should be done?
>
> Best regards,
> Sébastien
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Luc,
thanks for your detailed explanations attached to the MATH-659. I'm
worried about the changes I have applied to the code, now. Here is
what I've done. I've replaced the following line
{code}
return new Array2DRowRealMatrix(decomposition.getSolver().solve(b), false);
{code}
with
{code}
RealMatrix x = decomposition.getSolver().solve(new
Array2DRowRealMatrix(b, false));
return new Array2DRowRealMatrix(x.getData(), false);
{code}

decomposition is in fact an instance of QRDecompositionImpl.Solver,
whose method solve(RealMatrix) returns a BlockRealMatrix, not an
Array2DRowRealMatrix, hence the ugly last line. This code seems to be
correct (unit tests still pass), but I'm worried about the efficiency,
especially if initializeHighOrderDerivatives is called very often.
What do you think should be done?

Best regards,
Sébastien

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

Re: [math] use the more general super-type RealMatrix in place of Array2DRowRealMatrix in package ode?

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 08/09/2011 08:07, Sébastien Brisard a écrit :
> Hi,
> this question was raised while working on MATH-659, which I quote
> {quote}
> In rev1166533, I removed the call to this method in
> AdamsNordsieckTransformer.initializeHighOrderDerivatives(double,
> double[], double[][], double[][]). This raises one important question.
> All across the package o.a.c.m.ode, matrix variables are explicitely
> declared as Array2DRowRealMatrix (as opposed to RealMatrix). This
> leads to an ugly cast (see the end of the presently modified method).
> I was wondering how necessary that was. Would it be thinkable to use
> the most general supertype for all matrices?
> {quote}
> I should mention I have no knowledge at all about this package, so I
> do apologize if it is naive.

Sorry I did not see the message here. I have provided more info on the 
issue. If you want to continue discuss on it you are right, the mailing 
list is a better place for discussion.

Luc

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


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