bugGNU Octave - Bugs: bug #50463, gsvd implementation uses functions...

 
 

bug #50463: gsvd implementation uses functions removed from lapack >= 3.6.0

Submitted by:  Carlo de Falco <cdf>
Submitted on:  Sun Mar 5 18:50:25 2017  
 
Category: Configuration and Build SystemSeverity: 5 - Blocker
Priority: 5 - NormalItem Group: Build Failure
Status: In ProgressAssigned to: None
Originator Name: Open/Closed: Open
Release: devOperating System: Any

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon Jun 5 14:28:26 2017, comment #12:

Ping!

I hit this also when building in Mageia-6. The patch in comment #10 helps.

Philip Nienhuis <philipnienhuis>
Project Member
Wed May 3 17:09:01 2017, comment #11:

Great. I'll review whenever it is available.

Rik <rik5>
Project Administrator
Wed May 3 13:33:10 2017, comment #10:

Here's the updated patch.

It was so simple to update it that I wonder why mercurial
could not work it out automatically ...

This still replaces dggsvd by dggsvd3 unconditionally,
I'll now try the approach suggested by Rik for checking
whether dggsvd3 is actually available before invoking it.

c.

(file #40572)

Carlo de Falco <cdf>
Project Member
Fri Apr 28 23:25:47 2017, comment #9:

What was the resolution on whether this should be a runtime or compile time check for DGGSVD3?

I do see Mike's point about people switching BLAS libraries in and out. The MXE installer for Windows, for example, gives the user the option to use either OpenBLAS or refBLAS.

If it is a runtime test, then maybe promote the test to a static variable that is initialized just once at startup?

For example,

Also, this looks good, but is there a way to make this portable to other OS like Windows which might not have dlsym? Could you do a try/catch block?

Rik <rik5>
Project Administrator
Fri Apr 28 15:59:40 2017, comment #8:

as of hg id 855122b993da the patch below does not apply anymore.

I'll be looking at how to adapt the patch, unless anyone else knows straight away how to fix it ...

Carlo de Falco <cdf>
Project Member
Sun Mar 26 14:16:13 2017, comment #7:

Mike,

I do see the point in your proposed approach.

Please notice, though, that, as noted in comment #1 , the change
is indeed in netlib lapack API and not only in OpenBlas:

http://www.netlib.org/lapack/lapack-3.6.0.html#_3_lapack_3_6_0_what_8217_s_new

It also seems the problem had already been noticed by Nir and Carnë who were supervising a related GSOC project :

https://bitbucket.org/BarbaraLocsi/octave
http://octave.1599824.n4.nabble.com/Deprecated-LAPACK-routines-GSVD-tp4679040.html

Also it appears that scipy have been discussing the same issue:

https://github.com/scipy/scipy/issues/5266

So assuming the BLAS/LAPACK API to be stable is not really such a safe choice anyway ...

c.

Carlo de Falco <cdf>
Project Member
Sat Mar 25 21:27:34 2017, comment #6:

I don't have a patch, but a safer alternative might be to detect whether the LAPACK library that Octave has already loaded has the newer function or not.

Something like

and of course need to adapt this to other operating systems.

Mike Miller <mtmiller>
Project Administrator
Sat Mar 25 21:20:41 2017, comment #5:

Not saying this is a bad change, but this has a possibility of unintended consequences. Until now, Octave was built against the BLAS / LAPACK stable API that hasn't changed in decades. One could build Octave against standard BLAS and LAPACK, and then switch to either ATLAS or OpenBLAS at run time.

If this change is applied, it's possible to build Octave against an old BLAS / LAPACK, and then try to use OpenBLAS at run time, which may exhibit the same run time error. Or it may be possible to build Octave against a LAPACK that does have the xGGSVD3 functions, and switch to a different one at run time that doesn't, resulting in a similar run time error.

On my system, NETLIB, ATLAS, and OpenBLAS all have the xGGSVD3 functions, and I don't know how likely it is to find a system that has a mix of libraries with and without this function, but it's possible.

Mike Miller <mtmiller>
Project Administrator
Sun Mar 19 13:37:23 2017, comment #4:

Thanks Rik,
I'll combine the two patches tomorrow on the train to Geneve,
hope we can push it during the code sprints.
c.

Carlo de Falco <cdf>
Project Member
Sun Mar 19 05:47:54 2017, comment #3:

@Carlo: Attached is a small piece of detection code for configure.ac which will set the #define HAVE_DDGSVD3 if it is available in the LAPACK library on your system.

This should be combined with your existing patch. Everywhere in the code where there is a difference between DDGSVD3 and DDGSVD use this construct:

(file #40023)

Rik <rik5>
Project Administrator
Tue Mar 7 22:29:55 2017, comment #2:

Actually the reason why the previous patch did not
work was just a stupid mistake from my side.

The new patch build and passes all tests on my system.

Still needs the autoconf part to be complete,
I could really use some help for that.

(file #39931)

Carlo de Falco <cdf>
Project Member
Tue Mar 7 10:38:16 2017, comment #1:

I changed the title to include the minimum version of lapack needed to show the issue, which does not occur with the openblas implementation only.

I am still investigating why my patch does not work, even
if it did work it would still need to include autoconf tests
to determine the available version of ggsvd functions in
order to preserve backwards compatibility.

Carlo de Falco <cdf>
Project Member
Sun Mar 5 18:50:25 2017, original submission:

Hi,

I'm building the default branch on macOS 10.12.3
using maports for dependencies and openblas for
blas/lapack.

The build fails at the linking stage with the error

Looking at openblas sources, it appears that cggsvd is
deprecated and replaced by cggsvd3.

The attached patch solves the build problem for me but fails
at runtime with error:

Carlo de Falco <cdf>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #40572:  open_sJ86Ow6X.txt added by cdf (13kB - text/plain)
file #40023:  config.ac.diff added by rik5 (948B - application/x-download)
file #39931:  open_1VYMi0ZP.txt added by cdf (13kB - text/plain)
file #39896:  open_F5jqLbmL.txt added by cdf (13kB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Updated the item)
  • -unavailable- added by cdf (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed May 3 13:33:10 2017cdfAttached File-=>Added open_sJ86Ow6X.txt, #40572
    Fri Apr 28 23:25:47 2017rik5Operating SystemMac OS=>Any
    Sun Mar 19 05:48:58 2017rik5StatusNone=>In Progress
    Sun Mar 19 05:47:54 2017rik5Attached File-=>Added config.ac.diff, #40023
    Tue Mar 7 22:30:23 2017cdfSummarycggsvd implementation uses functions removed from lapack >= 3.6.0=>gsvd implementation uses functions removed from lapack >= 3.6.0
    Tue Mar 7 22:29:55 2017cdfAttached File-=>Added open_1VYMi0ZP.txt, #39931
    Tue Mar 7 10:38:16 2017cdfSeverity3 - Normal=>5 - Blocker
      Summarylinking fails as cggsvd deprecated in -lopenblas =>cggsvd implementation uses functions removed from lapack >= 3.6.0
    Sun Mar 5 18:50:25 2017cdfAttached File-=>Added open_F5jqLbmL.txt, #39896

    Back to the top


    Powered by Savane 3.1-cleanup1