[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #53140] Solution of a system of linear equatio

From: Rik
Subject: [Octave-bug-tracker] [bug #53140] Solution of a system of linear equations takes forever and hurts OS performance.
Date: Mon, 19 Feb 2018 14:39:15 -0500 (EST)
User-agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

Update of bug #53140 (project octave):

                  Status:               Confirmed => Ready For Test         


Follow-up Comment #13:

The problem seems to have been an assumption that floating point arithmetic
would be used for mathematical expressions.  But because all operands were
integers there was no need for the compiler to promote any integer to double. 
When performing the calculation with integers the result was a value which
exceeded the storage size of octave_index_type (signed int), and that resulted
in a negative number for the size.

Here is how I recoded it.

   // Resize the sparse matrix
-  octave_idx_type sz = x_nz * (b_nc - j) / b_nc;
-  sz = (sz > 10 ? sz : 10) + x_nz;
+  octave_idx_type sz;
+  sz = (static_cast<double> (b_nc) - j) / b_nc
+       * x_nz;
+  sz = x_nz + (sz > 100 ? sz : 100);
   retval.change_capacity (sz);

I tried to show more clearly that a fraction of the current number of non-zero
elements x_nz is added to x_nz to determine the new reserved storage space.

I only cast one of the operands to a double and then relied on the implicit
compiler rules.  However, if it seems clearer we could also code this using
static_casts on every value which we actually want to be double.  Or we could
use the double constructor explicitly.

I also changed the default increase in size from 10 elements to 100 elements. 
10 double values is ~80 bytes which is pretty small given most modern
machine's installed memory.  Even 100 elements is ~800B or about 1kB which is
also pretty small.  As an example, I found that the smallest increase in size
using the problem matrix was ~8,000 elements.

I checked this change in on the stable branch here

Marking as Ready for Test.


Reply to this item at:


  Message sent via/by Savannah

reply via email to

[Prev in Thread] Current Thread [Next in Thread]