bugGNU Octave - Bugs: bug #55950, building default with sundials 4.x...

 
 

bug #55950: building default with sundials 4.x fails to detect KLU in configure

Submitter:  Mike Miller <mtmiller>
Submitted:  Mon 18 Mar 2019 06:41:25 PM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 19 Mar 2019 04:39:06 PM UTC, comment #10: 

Carlo:  I wanted to use the OCTAVE_CHECK_LIB macro and thought it was OK to just check for suitesparse/klu.h and I didn't fully understand the dependency issues that OCTAVE_CHECK_LIB can't handled (one header file that requires another to be included first).  In any case, I think it is all fixed now.

John W. Eaton <jwe>
Group administrator
Tue 19 Mar 2019 04:36:28 PM UTC, comment #9: 

@cdf Nothing was removed, this is in addition to what was there previously.

When you first committed this change, it was only on the test for the SUNKLU function. Now there is also a test for whether the sunlinsol_klu.h header file can be included, without a test program, and that needed the additional logic also.

I would appreciate any feedback you have on bug #55937 to make sure I understand these tests properly before I go hacking this all up again.

Mike Miller <mtmiller>
Group Member
Tue 19 Mar 2019 03:59:38 PM UTC, comment #8: 

still I am a bit curious, why was the check for klu in
multiple locations first removed and then reintroduced?

Carlo de Falco <cdf>
Group Member
Tue 19 Mar 2019 11:41:56 AM UTC, comment #7: 

OK, sorry ...
I see now that

http://hg.savannah.gnu.org/hgweb/octave/rev/58e300954518

brought back the check for additional locations,
I should have read all the discussion before posting.


Carlo de Falco <cdf>
Group Member
Tue 19 Mar 2019 11:40:34 AM UTC, comment #6: 



Hi,

Actually klu.h is not necessarily in suitesparse/klu.h
at least it is not so in my system.

http://hg.savannah.gnu.org/hgweb/octave/rev/a1acd11bad19

used to have


+         #if defined (HAVE_KLU_H)
+         #include <klu.h>
+         #endif
+         #if defined (HAVE_KLU_KLU_H)
+         #include <klu/klu.h>
+         #endif
+         #if defined (HAVE_SUITESPARSE_KLU_H)
+         #include <suitesparse/klu.h>
+         #endif
+         #if defined (HAVE_UFPARSE_KLU_H)
+         #include <ufsparse/klu.h>
+         #endif



so, why does

http://hg.savannah.gnu.org/hgweb/octave/rev/cfa2a65c9fdc

only check for suitesparse/klu.h ?



Carlo de Falco <cdf>
Group Member
Mon 18 Mar 2019 10:22:45 PM UTC, comment #5: 

Thanks, this works for me now. Hopefully I'll get some time to work on bug #55937 and not break all of this again.

Mike Miller <mtmiller>
Group Member
Mon 18 Mar 2019 09:09:41 PM UTC, comment #4: 

I pushed this additional change:

http://hg.savannah.gnu.org/hgweb/octave/rev/58e300954518

Yeah, it's probably OK to not look for ufsparse/klu.h so if you want to remove that from the configure checks and sources, go ahead.

John W. Eaton <jwe>
Group administrator
Mon 18 Mar 2019 08:41:24 PM UTC, comment #3: 

Side note: should we stop checking for 'ufsparse' these days?

Mike Miller <mtmiller>
Group Member
Mon 18 Mar 2019 08:20:07 PM UTC, comment #2: 

I can retest this a little bit later today.

I think this needs more include variants of klu.h, though. See how klu.h is included in '__ode15__.cc', I think we need to mirror those same possibilities in configure.

https://hg.savannah.gnu.org/hgweb/octave/file/cfa2a65c9fdc/libinterp/dldfcn/__ode15__.cc#l64

Mike Miller <mtmiller>
Group Member
Mon 18 Mar 2019 07:55:54 PM UTC, comment #1: 

I pushed the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/cfa2a65c9fdc

It seems to work for me with Sundials 4.1.0 without having to add -I/usr/include/suitesparse to CPPFLAGS.  Building with Sundials 3.x also works, but I think the -I flag is still needed for that version.

John W. Eaton <jwe>
Group administrator
Mon 18 Mar 2019 06:41:25 PM UTC, original submission:  

I have finally gotten time to build and install my own copy of sundials 4.1. When running configure with CPPFLAGS and LDFLAGS pointing to this copy of sundials, configure finds it, but does not automatically detect that the library was built with KLU support. It definitely is, the only problem still seems to be the link to the klu.h header file.

Excerpt from config.log


configure:70899: checking sunlinsol/sunlinsol_klu.h usability
configure:70899: gcc -c -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -pthread -fopenmp   -D_FORTIFY_SOURCE=2 -I/home/mike/sd/include conftest.c >&5
In file included from conftest.c:604:
/home/mike/sd/include/sunlinsol/sunlinsol_klu.h:34:10: fatal error: klu.h: No such file or directory
 #include <klu.h>
          ^~~~~~~


But sundials 4 was adapted to allow the application to include klu.h in its own way and not depend on the path being exactly klu.h. So configure needs to test for the header file this way for the feature tests to pass.

If I fudge the configure feature test results by passing the following five cache variables that I know should be passing on my system


ac_cv_func_SUNKLU=yes
ac_cv_func_SUNLinSol_KLU=yes
ac_cv_header_sunlinsol_sunlinsol_klu_h=yes
octave_cv_lib_sundials_sunlinsolklu=yes
octave_cv_sundials_sunlinsol_klu=yes


then configure passes with all sundials features enabled for the v4 API, and 'make' succeeds and all tests pass, no features disabled.

The point here is to avoid requiring the user to pass '-I/usr/include/suitesparse' when building Octave with current versions of sundials, configure needs to include klu.h under different alternative names in the sundials tests just like it does in the '__ode15__.cc' source file.

Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by cdf (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  •  

    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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-03-18 mtmiller StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2019-03-18 jwe StatusNone Ready For Test
    2019-03-18 mtmiller Carbon-CopyRemoved 80942 -

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code