bugGNU Octave - Bugs: bug #44722, linsolve.m: Rand unit test fails...

 
 

bug #44722: linsolve.m: Rand unit test fails randomly...

Submitter:  Julien Bect <jbect>
Submitted:  Sat 04 Apr 2015 04:52:55 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.0.0 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 02 Jun 2015 04:42:49 PM UTC, comment #11: 

Retagging release from 4.0.0-rc2 to 4.0.0.

John W. Eaton <jwe>
Group administrator
Mon 06 Apr 2015 11:22:27 AM UTC, comment #10: 
Rik <rik5>
Group administrator
Mon 06 Apr 2015 06:44:44 AM UTC, comment #9: 

Of course, if the goal is to check that the same algorithm is called both times under the hood, then a zero-tolerance test is appropriate. Perhaps a comment could help clarifiy that this is indeed the purpose of this unit test ?

The proposed patch works for me.

Julien Bect <jbect>
Mon 06 Apr 2015 12:15:08 AM UTC, comment #8: 

The zero tolerance was useful in catching that we were using a different algorithm between linsolve and the expected result in assert.  I've re-written linsolve to not perform the transpose of A in Octave, and instead forward the transpose flag to BLAS.  This is what we already do if you write


A' \ b


at the prompt.  This yields a 40% speed improvement in linsolve and there shouldn't be a BIST failure anymore because linsolve and the interpreter are using the same algorithm.

A changeset for test is attached.  Please try it and if it works I'll commit it before the 4.0 release.



(file #33557)

Rik <rik5>
Group administrator
Sat 04 Apr 2015 09:30:08 PM UTC, comment #7: 

Rik:  6 runs of the script in file #33538 all pass
on Fedora 21 x86_64

Michael Godfrey <godfrey>
Group Member
Sat 04 Apr 2015 08:39:35 PM UTC, comment #6: 

@Julien: Now it makes sense (comment #5).  Octave used to interpret


A' \ b

as

(A') \ b


The transpose was done in Octave and then the work of the left division operator was left to BLAS/LAPACK.

This was completely inefficient as BLAS/LAPACK have options for indicating whether a matrix is being entered in a transposed form or not.  Now we just call BLAS and give the correct options, but it could lead to a small difference in calculation.


Rik <rik5>
Group administrator
Sat 04 Apr 2015 07:16:28 PM UTC, comment #5: 

Another fact: on the same machine, I have never seen this test fail with my previous Octave build (it was somewhere around 3.8.2, on the stable branch).

My guess is that under the old build the two ways of computing A' \ b were actually exactly the same, whereas now, for some reason, different algorithms are being used. Does it make sense ?

Julien Bect <jbect>
Sat 04 Apr 2015 07:11:42 PM UTC, comment #4: 

Actually, with the integer-valued matrices I used in the patch, I get an exact equality. So, even without the tolerance (10 eps) that I introduced, the test passes.

However, unless the goal is to check that the same algorithm is used for both sides of the assertion, I don't think that it's reasonable to use a zero tolerance.


Julien Bect <jbect>
Sat 04 Apr 2015 06:07:22 PM UTC, comment #3: 

If you pass a negative tolerance, it's treated as relative tolerance. I think that's better for this case.

I also wish we could be very tight about tolerances and not simply increase them when tests fail. I know the numerical error analysis of what the proper tolerance should be is tricky, but it's not impossible.

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Sat 04 Apr 2015 06:05:35 PM UTC, comment #2: 

Before patching I'd like to confirm that this is a general problem.  This hasn't come up before which may mean it was just buried or it may mean there is something unique to your setup.

I'm attaching lintst.m which runs the linsolve tests 100 times and reports any failures.  I never get any on a Linux system or Windows XP virtual machine.

If other coders read this report I would appreciate them testing and reporting back.

(file #33538)

Rik <rik5>
Group administrator
Sat 04 Apr 2015 04:55:14 PM UTC, comment #1: 

Here is a patch that fixes the problem for me.

I have inserted a tolerance for the test (10 *eps) and also replaced the random data used in test by deterministic data.

(file #33534)

Julien Bect <jbect>
Sat 04 Apr 2015 04:52:55 PM UTC, original submission:  

I get random test failures for linsolve on a Debian wheezy (7.8) x86_64 machine:

>>>>> processing /usr/local/src/octave/default/scripts/linear-algebra/linsolve.m

*** test
 n = 4;
 A = triu (rand (n));
 x = rand (n, 1);
 b = A' * x;
 opts.UT = true;
 opts.TRANSA = true;
 assert (linsolve (A, b, opts), A' \ b);

!!!!! test failed
ASSERT errors for:  assert (linsolve (A, b, opts),A' \ b)

  Location  |  Observed  |  Expected  |  Reason
    (1)        0.42063      0.42063      Abs err 5.5511e-17 exceeds tol 0
    (2)        0.68722      0.68722      Abs err 4.1078e-15 exceeds tol 0
    (3)        0.25533      0.25533      Abs err 2.1094e-15 exceeds tol 0
    (4)        0.52907      0.52907      Abs err 7.3275e-15 exceeds tol 0

Julien Bect <jbect>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #33557:  linsolve.cset added by rik5 (2KiB - application/octet-stream)
file #33538:  lintst.m added by rik5 (103B - text/x-objcsrc)
file #33534:  linsolve.patch added by jbect (789B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by jbect (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-06-02 jwe Release4.0.0-rc2 4.0.0
    2015-04-06 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2015-04-06 rik5 Attached File- Added linsolve.cset, #33557
        StatusNone Ready For Test
    2015-04-04 rik5 Attached File- Added lintst.m, #33538
    2015-04-04 jbect Attached File- Added linsolve.patch, #33534

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code