You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Mark Thomas <ma...@apache.org> on 2011/09/07 12:39:37 UTC

Re: svn commit: r1166099 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java

On 07/09/2011 11:34, erans@apache.org wrote:
> Author: erans
> Date: Wed Sep  7 10:34:49 2011
> New Revision: 1166099
> 
> URL: http://svn.apache.org/viewvc?rev=1166099&view=rev
> Log:
> Added "final".
> Moved declaration of "sum" where it is used.

You might want to re-consider that. I suspect sum was declared outside
of the inner loops to save allocating memory for a new double on every
iteration of the loop. It may be worth moving the declaration outside
the outer loop too.

Mark


> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java?rev=1166099&r1=1166098&r2=1166099&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java Wed Sep  7 10:34:49 2011
> @@ -36,9 +36,9 @@ public class LUDecompositionImpl impleme
>      /** Default bound to determine effective singularity in LU decomposition. */
>      private static final double DEFAULT_TOO_SMALL = 1e-11;
>      /** Entries of LU decomposition. */
> -    private double lu[][];
> +    private final double lu[][];
>      /** Pivot permutation associated with LU decomposition */
> -    private int[] pivot;
> +    private final int[] pivot;
>      /** Parity of the permutation associated with the LU decomposition */
>      private boolean even;
>      /** Singularity indicator. */
> @@ -92,12 +92,10 @@ public class LUDecompositionImpl impleme
>          // Loop over columns
>          for (int col = 0; col < m; col++) {
>  
> -            double sum = 0;
> -
>              // upper
>              for (int row = 0; row < col; row++) {
>                  final double[] luRow = lu[row];
> -                sum = luRow[col];
> +                double sum = luRow[col];
>                  for (int i = 0; i < row; i++) {
>                      sum -= luRow[i] * lu[i][col];
>                  }
> @@ -109,7 +107,7 @@ public class LUDecompositionImpl impleme
>              double largest = Double.NEGATIVE_INFINITY;
>              for (int row = col; row < m; row++) {
>                  final double[] luRow = lu[row];
> -                sum = luRow[col];
> +                double sum = luRow[col];
>                  for (int i = 0; i < col; i++) {
>                      sum -= luRow[i] * lu[i][col];
>                  }
> 
> 


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


Re: svn commit: r1166099 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java

Posted by sebb <se...@gmail.com>.
On 7 September 2011 13:46, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> On Wed, Sep 07, 2011 at 01:16:03PM +0100, Mark Thomas wrote:
>> On 07/09/2011 12:49, Gilles Sadowski wrote:
>> > On Wed, Sep 07, 2011 at 11:39:37AM +0100, Mark Thomas wrote:
>> >> On 07/09/2011 11:34, erans@apache.org wrote:
>> >>> Author: erans
>> >>> Date: Wed Sep  7 10:34:49 2011
>> >>> New Revision: 1166099
>> >>>
>> >>> URL: http://svn.apache.org/viewvc?rev=1166099&view=rev
>> >>> Log:
>> >>> Added "final".
>> >>> Moved declaration of "sum" where it is used.
>> >>
>> >> You might want to re-consider that. I suspect sum was declared outside
>> >> of the inner loops to save allocating memory for a new double on every
>> >> iteration of the loop. It may be worth moving the declaration outside
>> >> the outer loop too.
>> >
>> > Reducing scope makes for clearer code and can lead to _more_ optimized code.
>>
>> Cleaner code, certainly.
>>
>> As far as optimisation goes I'm not so sure with a primitive. On further
>> research it looks like the performance should be identical. For an
>> "interesting" discussion on what exactly is going on I found this:
>> http://www.javalobby.org/java/forums/t16730.html?start=0
>
> So:
>  - Cleaner code: Yes
>  - Faster code: Maybe
>  - Higher memory usage: No
>
> Do we agree that the change is fine?

OK by me.

>
> Gilles
>
> ---------------------------------------------------------------------
> 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: svn commit: r1166099 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 07/09/2011 14:46, Gilles Sadowski a écrit :
> On Wed, Sep 07, 2011 at 01:16:03PM +0100, Mark Thomas wrote:
>> On 07/09/2011 12:49, Gilles Sadowski wrote:
>>> On Wed, Sep 07, 2011 at 11:39:37AM +0100, Mark Thomas wrote:
>>>> On 07/09/2011 11:34, erans@apache.org wrote:
>>>>> Author: erans
>>>>> Date: Wed Sep  7 10:34:49 2011
>>>>> New Revision: 1166099
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1166099&view=rev
>>>>> Log:
>>>>> Added "final".
>>>>> Moved declaration of "sum" where it is used.
>>>>
>>>> You might want to re-consider that. I suspect sum was declared outside
>>>> of the inner loops to save allocating memory for a new double on every
>>>> iteration of the loop. It may be worth moving the declaration outside
>>>> the outer loop too.
>>>
>>> Reducing scope makes for clearer code and can lead to _more_ optimized code.
>>
>> Cleaner code, certainly.
>>
>> As far as optimisation goes I'm not so sure with a primitive. On further
>> research it looks like the performance should be identical. For an
>> "interesting" discussion on what exactly is going on I found this:
>> http://www.javalobby.org/java/forums/t16730.html?start=0
>
> So:
>   - Cleaner code: Yes
>   - Faster code: Maybe
>   - Higher memory usage: No
>
> Do we agree that the change is fine?

Yes, I prefer to have a smaller scope too.

Luc

>
>
> Gilles
>
> ---------------------------------------------------------------------
> 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: svn commit: r1166099 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 07, 2011 at 01:16:03PM +0100, Mark Thomas wrote:
> On 07/09/2011 12:49, Gilles Sadowski wrote:
> > On Wed, Sep 07, 2011 at 11:39:37AM +0100, Mark Thomas wrote:
> >> On 07/09/2011 11:34, erans@apache.org wrote:
> >>> Author: erans
> >>> Date: Wed Sep  7 10:34:49 2011
> >>> New Revision: 1166099
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1166099&view=rev
> >>> Log:
> >>> Added "final".
> >>> Moved declaration of "sum" where it is used.
> >>
> >> You might want to re-consider that. I suspect sum was declared outside
> >> of the inner loops to save allocating memory for a new double on every
> >> iteration of the loop. It may be worth moving the declaration outside
> >> the outer loop too.
> > 
> > Reducing scope makes for clearer code and can lead to _more_ optimized code.
> 
> Cleaner code, certainly.
> 
> As far as optimisation goes I'm not so sure with a primitive. On further
> research it looks like the performance should be identical. For an
> "interesting" discussion on what exactly is going on I found this:
> http://www.javalobby.org/java/forums/t16730.html?start=0

So:
 - Cleaner code: Yes
 - Faster code: Maybe
 - Higher memory usage: No

Do we agree that the change is fine?


Gilles

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


Re: svn commit: r1166099 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java

Posted by Mark Thomas <ma...@apache.org>.
On 07/09/2011 12:49, Gilles Sadowski wrote:
> On Wed, Sep 07, 2011 at 11:39:37AM +0100, Mark Thomas wrote:
>> On 07/09/2011 11:34, erans@apache.org wrote:
>>> Author: erans
>>> Date: Wed Sep  7 10:34:49 2011
>>> New Revision: 1166099
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1166099&view=rev
>>> Log:
>>> Added "final".
>>> Moved declaration of "sum" where it is used.
>>
>> You might want to re-consider that. I suspect sum was declared outside
>> of the inner loops to save allocating memory for a new double on every
>> iteration of the loop. It may be worth moving the declaration outside
>> the outer loop too.
> 
> Reducing scope makes for clearer code and can lead to _more_ optimized code.

Cleaner code, certainly.

As far as optimisation goes I'm not so sure with a primitive. On further
research it looks like the performance should be identical. For an
"interesting" discussion on what exactly is going on I found this:
http://www.javalobby.org/java/forums/t16730.html?start=0

Mark

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


Re: svn commit: r1166099 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/LUDecompositionImpl.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 07, 2011 at 11:39:37AM +0100, Mark Thomas wrote:
> On 07/09/2011 11:34, erans@apache.org wrote:
> > Author: erans
> > Date: Wed Sep  7 10:34:49 2011
> > New Revision: 1166099
> > 
> > URL: http://svn.apache.org/viewvc?rev=1166099&view=rev
> > Log:
> > Added "final".
> > Moved declaration of "sum" where it is used.
> 
> You might want to re-consider that. I suspect sum was declared outside
> of the inner loops to save allocating memory for a new double on every
> iteration of the loop. It may be worth moving the declaration outside
> the outer loop too.

Reducing scope makes for clearer code and can lead to _more_ optimized code.


Gilles

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