bugGNU Octave - Bugs: bug #55937, building default with sundials 2.x...

 
 

bug #55937: building default with sundials 2.x fails in make, configure should disable sundials

Submitter:  Mike Miller <mtmiller>
Submitted:  Sat 16 Mar 2019 08:23:37 PM UTC
   
 
Category:  Configuration and Build System Severity:  4 - Important
Priority:  5 - Normal Item Group:  Build Failure
Status:  Fixed Assigned to:  mtmiller
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 02 Apr 2019 10:34:23 PM UTC, comment #24: 

Ok, I have now tested again with

  • SUNDIALS 2.7, correctly warns only once that SUNDIALS is not compatible and will be disabled
  • SUNDIALS 3.1 without -I/usr/include/suitesparse, correctly warns that KLU isn't working, sparse functionality disabled
  • SUNDIALS 3.1 with -I/usr/include/suitesparse, no warnings, all features enabled, 'HAVE_SUNKLU' is defined
  • SUNDIALS 4.1, no warnings, all features enabled, 'HAVE_SUNKLU' and 'HAVE_SUNLINSOL_KLU' are both defined
Mike Miller <mtmiller>
Group Member
Tue 02 Apr 2019 10:04:40 PM UTC, comment #23: 

Pushed on the default branch

https://hg.savannah.gnu.org/hgweb/octave/rev/b37c1656aa42

I have not exhaustively re-tested all possible combinations of SUNDIALS, but I will try to do that once more to see if it all still works.

Mike Miller <mtmiller>
Group Member
Mon 01 Apr 2019 10:11:45 PM UTC, comment #22: 

@Mike: Seems okay to push the second bit of your patch from comment #20, and then close this report.

Rik <rik5>
Group administrator
Fri 29 Mar 2019 04:07:39 PM UTC, comment #21: 

@Mike: I can confirm that your patch fixes the issue. With Sundials 2.7 installed, I get the following warning at configure time:


configure: WARNING: SUNDIALS libraries do not provide an API that is compatible with Octave.  The solvers ode15i and ode15s will be disabled.


and the build completes successfully (without ode15i/s).

Pantxo Diribarne <pantxo>
Group Member
Thu 28 Mar 2019 05:13:29 AM UTC, comment #20: 

Ok, can you test the attached patch? I think it works for me, tested again with all combinations. I think it's detecting the SUNKLU function when it should now, or the SUNLinSol_KLU function with version 4. And I don't get duplicate warning messages with sundials 2.7 now.

Does this work for you? Can we please be done with all of these sundials problems soon?

(file #46653)

Mike Miller <mtmiller>
Group Member
Thu 28 Mar 2019 04:48:28 AM UTC, comment #19: 

Working on yet another fix now, will post a patch here for you to test.

Mike Miller <mtmiller>
Group Member
Thu 28 Mar 2019 04:37:32 AM UTC, comment #18: 

Ok, I see the difference, on Debian with SUNDIALS 3.1.2 and using -I/usr/include/suitesparse, with my original ordering it does not detect the 'SUNKLU' function specifically, no error or warning is printed, it would just silently disable that feature at build time.

Somehow we need to do the optional tests for KLU features after testing whether SUNDIALS will work at all, but unfortunately the same macro that tests for version 3 vs version 4 API functions also tests for KLU functions. Maybe we can move just the tests for the 'SUNLinSol_KLU' and 'SUNKLU' functions into the 'OCTAVE_CHECK_SUNDIALS_SUNLINSOL_KLU' macro?

Mike Miller <mtmiller>
Group Member
Thu 28 Mar 2019 04:11:02 AM UTC, comment #17: 


> checking whether SUNDIALS API provides the necessary functions... no


And I just realized I wasn't really done coming up with a suitable message here, I am open to suggestions about how to describe this test for the version 3 or version 4 API functions.

Mike Miller <mtmiller>
Group Member
Thu 28 Mar 2019 04:03:54 AM UTC, comment #16: 

Really? I specifically tested that, also Debian with SUNDIALS 3.1.2, and everything seemed to be working for me. Did you see a problem with configure or only when you run make?

The reason I put the checks in that order was specifically to test for required features first that may short-circuit the following tests. With your change, I now get this on a system with SUNDIALS 2.7:


checking for sunlinsol/sunlinsol_klu.h... no
checking for SUNKLU in -lsundials_sunlinsolklu... no
configure: WARNING: SUNDIALS IDA library not configured with SUNLINSOL_KLU or sunlinksol_klu.h is not usable.  The solvers ode15i and ode15s will not support the sparse Jacobian feature.
checking for IDASetJacFn... no
checking for IDASetLinearSolver... no
checking for SUNLinSol_Dense... no
checking for SUNLinSol_KLU... no
checking ida/ida_direct.h usability... yes
checking ida/ida_direct.h presence... yes
checking for ida/ida_direct.h... yes
checking ida_direct.h usability... no
checking ida_direct.h presence... no
checking for ida_direct.h... no
checking for IDADlsSetJacFn... no
checking for IDADlsSetLinearSolver... no
checking for SUNDenseLinearSolver... no
checking for SUNKLU... no
checking whether SUNDIALS API provides the necessary functions... no
configure: WARNING: SUNDIALS libraries do not provide an API that is compatible with Octave.  The solvers ode15i and ode15s will be disabled.


And then again in the configure summary at the end


configure: WARNING: SUNDIALS IDA library not configured with SUNLINSOL_KLU or sunlinksol_klu.h is not usable.  The solvers ode15i and ode15s will not support the sparse Jacobian feature.
configure: WARNING: SUNDIALS libraries do not provide an API that is compatible with Octave.  The solvers ode15i and ode15s will be disabled.


I would really like to avoid these duplicate warnings if we could.

Mike Miller <mtmiller>
Group Member
Thu 28 Mar 2019 03:44:56 AM UTC, comment #15: 

I needed the following additional change to make the build work again with Sundials 3.1 on a Debian system.  Otherwise, the check for SUNKLU fails.

http://hg.savannah.gnu.org/hgweb/octave/rev/5bd4ffb55b1b

John W. Eaton <jwe>
Group administrator
Wed 27 Mar 2019 10:52:38 PM UTC, comment #14: 

This is fixed now on the default branch with this change


I also pushed a followup change to make the SUNDIALS detection fail fast and avoid printing 2 or 3 warning messages if several feature tests fail


On the default branch, I can now configure and build with SUNDIALS version 2.7.0 (configure disables automatically), version 3.1.2 without -I/usr/include/suitesparse (configure warns about KLU not found), version 3.1.2 with -I/usr/include/suitesparse (no warning, all features enabled), and version 4.1.0 (no warning, all features enabled). I also tested building with '--without-sundials_nvecserial', and it avoids unnecessary tests for optional features because it already knows that the library is missing.

Mike Miller <mtmiller>
Group Member
Wed 27 Mar 2019 08:55:24 PM UTC, comment #13: 

I have a fix for this, testing with SUNDIALS 2.7, 3.1 without -I/usr/include/suitesparse, 3.1 with -I/usr/include/suitesparse, and 4.1.

Now I'm just trying to clean up redundant warning messages for various SUNDIALS failures.

This is all on default, Octave 5.1 is not affected by this task, nothing here is holding up 5.2.

Mike Miller <mtmiller>
Group Member
Wed 27 Mar 2019 07:07:32 PM UTC, comment #12: 

No, this bug is still open, I'm looking at configure today, but may take a little while to reorganize.

Mike Miller <mtmiller>
Group Member
Wed 27 Mar 2019 06:44:10 PM UTC, comment #11: 

The overlinking fix for bug #55956 was applied.  Can this bug also be closed?

Rik <rik5>
Group administrator
Tue 19 Mar 2019 06:57:49 PM UTC, comment #10: 

See bug #55956 for solution to the overlinking bug, made on stable and merged to default.

Mike Miller <mtmiller>
Group Member
Tue 19 Mar 2019 05:21:21 PM UTC, comment #9: 

Fixing a configuration and build issue on stable seems OK to me.

John W. Eaton <jwe>
Group administrator
Tue 19 Mar 2019 05:09:25 PM UTC, comment #8: 

Yeah, agree with both of you. When I said "builds cleanly even if the user has sundials 2", I meant to fix the false positive that Carlo mentions, still a problem right now.

I have a fix for the overlinking problem that I can commit to stable and merge to default, it's very simple and should be safe for stable, but I will happily submit as a patch for review or avoid stable altogether if you think it's too dangerous.

Mike Miller <mtmiller>
Group Member
Tue 19 Mar 2019 05:02:13 PM UTC, comment #7: 

AFAIK The current situation is:

Octave 4 and Octave 5  build correctly with any version of sundials

if sundials is 2.7 ode15 is activated, for any other version it is correctly detected as non compatible and deactivated

Octave 6 seems to detect correctly either sundials 3 or sundials 4 and seems to set all flags correctly

last time I tried to build Octave 6 with sundials 2.7, configure detected sundials as available but then the build failed, which is consistent with your observation below.

The only thing that definitely needs fixing is the false positive detection of sundials 2.7 in
Octave 6. Octave 6 should definitely disable ode15 if sundials is older than version 3.

My further suggestion would be to also drop support for sundials 3 to avoid extra complications
related to different APIs.

Carlo de Falco <cdf>
Group Member
Tue 19 Mar 2019 04:43:02 PM UTC, comment #6: 

Currently, I think Octave 6 will work with either Sundials 3 or 4.

I don't personally care about earlier versions and would only recommend making Octave work with Sundials 2.7 if it is easily done and/or necessary because we are likely to find that many distributions currently in use don't have Sundials 3.

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

Thank you. My intent is to make sure Octave 6 configures and builds cleanly even if the user has sundials 2 installed, which means we need to account for that case and properly disable everything, including overlinking the sundials libraries. I also plan on fixing that last part on stable (the '__ode15__.oct' file should not be linked with -lsundials libs if HAVE_SUNDIALS is not set).

Mike Miller <mtmiller>
Group Member
Tue 19 Mar 2019 04:28:37 PM UTC, comment #4: 

@mtmiller,

Re comment #2: yes,

either IDASetJacFn or IDADlsSetJacFn
and either IDASetLinearSolver or IDADlsSetLinearSolver
and either SUNLinSol_Dense or SUNDenseLinearSolver

are required.

If any of these functions is missing sundials should be disabled altogether.

Re the note at the end of comment #3 :

as for the check for current and deprecated interfaces my intention was to
only accept the new interfaces (available in sundials > 4.0.1),
but I think jwe wants to also support some prior versions as that is what
is currently distributed with debian.

my personal opinion would be that it makes sense to spend as little as
possible effort in supporting sundials 3 because by the time Octave 6 is released
sundials 4 will probably be more widely available as a package.

Carlo de Falco <cdf>
Group Member
Mon 18 Mar 2019 05:46:48 PM UTC, comment #3: 

Oops, forgot to add @cdf in Cc, done now.

Carlo, does this look right to you? It looks to me like we need to reorganize the checks in configure. Specifically, configure should not define HAVE_SUNDIALS until after it tests for the current and deprecated interfaces.

The section for "Current interface" and "Deprecated interface" seems like an afterthought, maybe it should be moved into a new OCTAVE_CHECK_SUNDIALS_SUNLINSOL_API macro and called with the other CHECKs? The possible outcomes are "current", "deprecated", and "neither", and for the latter sundials should not be used.

Mike Miller <mtmiller>
Group Member
Sat 16 Mar 2019 08:42:20 PM UTC, comment #2: 

From the configure output, these are the version 4 API functions


checking for IDASetJacFn... no
checking for IDASetLinearSolver... no
checking for SUNLinSol_Dense... no
checking for SUNLinSol_KLU... no


and these are the corresponding version 3 API functions


checking for IDADlsSetJacFn... no
checking for IDADlsSetLinearSolver... no
checking for SUNDenseLinearSolver... no
checking for SUNKLU... no


@cdf Does this look right to you? We have to detect one or the other of these sets of functions (except for KLU, which is optional). If they are all "no" then HAVE_SUNDIALS should not be set.

Mike Miller <mtmiller>
Group Member
Sat 16 Mar 2019 08:38:04 PM UTC, comment #1: 

I looked into '__ode15__.cc' a bit more, and I see how the conditionals are set up to work with the version 3 and version 4 APIs. So for example, sundials needs to provide either of the 'SUNLinSol_Dense' or 'SUNDenseLinearSolver' functions, both serve the same purpose. But if neither function is found, then sundials is not a compatible version, and we should detect that in configure.

Mike Miller <mtmiller>
Group Member
Sat 16 Mar 2019 08:23:37 PM UTC, original submission:  

The default branch only supports the new sundials API in version 3.x and later.

Trying to build the default branch on a system with sundials 2.x now, configure thinks that it found a suitable version of sundials, the feature tests think that it is good enough, but make fails with an error. This should be detected in configure and sundials disabled completely.

Here is what configure puts out with sundials 2.7.0


checking for nvector/nvector_serial.h... yes
checking for N_VNew_Serial in -lsundials_nvecserial... yes
checking for ida/ida.h... yes
checking for IDAInit in -lsundials_ida... yes
checking whether SUNDIALS IDA is configured with double precision realtype... yes
checking for sunlinsol/sunlinsol_dense.h... no
checking whether SUNDIALS IDA includes the SUNLINSOL_DENSE linear solver... yes
checking for sunlinsol/sunlinsol_klu.h... no
configure: WARNING: SUNDIALS IDA library not configured with SUNLINSOL_KLU or sunlinksol_klu.h is not usable; ode15i and ode15s will not support the sparse Jacobian feature
checking for IDASetJacFn... no
checking for IDASetLinearSolver... no
checking for SUNLinSol_Dense... no
checking for SUNLinSol_KLU... no
checking for ida/ida_direct.h... yes
checking for ida_direct.h... no
checking for IDADlsSetJacFn... no
checking for IDADlsSetLinearSolver... no
checking for SUNDenseLinearSolver... no
checking for SUNKLU... no


With all of those feature tests, and all of those "no"s, is there enough information here already to disable sundials?
If not, it looks like it should be very easy to test for the SUNMatrix or SUNLinearSolver typedefs and if they don't exist disable sundials.

Here are the compilation errors


  CXX      libinterp/dldfcn/__ode15___la-__ode15__.lo
../libinterp/dldfcn/__ode15__.cc:264:39: error: 'SUNMatrix' has not been declared
               N_Vector yyp, N_Vector, SUNMatrix JJ, void *user_data,
                                       ^~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:274:49: error: 'SUNMatrix' has not been declared
                    N_Vector& yy, N_Vector& yyp, SUNMatrix& JJ);
                                                 ^~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:353:5: error: 'SUNMatrix' does not name a type; did you mean 'Matrix'?
     SUNMatrix sunJacMatrix;
     ^~~~~~~~~
     Matrix
../libinterp/dldfcn/__ode15__.cc:354:5: error: 'SUNLinearSolver' does not name a type
     SUNLinearSolver sunLinearSolver;
     ^~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc: In constructor 'octave::IDA::IDA()':
../libinterp/dldfcn/__ode15__.cc:169:9: error: class 'octave::IDA' does not have any field named 'sunJacMatrix'
         sunJacMatrix (nullptr), sunLinearSolver (nullptr)
         ^~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:169:33: error: class 'octave::IDA' does not have any field named 'sunLinearSolver'
         sunJacMatrix (nullptr), sunLinearSolver (nullptr)
                                 ^~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc: In constructor 'octave::IDA::IDA(realtype, ColumnVector, ColumnVector, const octave_value&, octave::IDA::DAERHSFuncIDA)':
../libinterp/dldfcn/__ode15__.cc:180:9: error: class 'octave::IDA' does not have any field named 'sunJacMatrix'
         sunJacMatrix (nullptr), sunLinearSolver (nullptr)
         ^~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:180:33: error: class 'octave::IDA' does not have any field named 'sunLinearSolver'
         sunJacMatrix (nullptr), sunLinearSolver (nullptr)
                                 ^~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc: In destructor 'octave::IDA::~IDA()':
../libinterp/dldfcn/__ode15__.cc:187:21: error: 'sunLinearSolver' was not declared in this scope
       SUNLinSolFree(sunLinearSolver);
                     ^~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:187:7: error: 'SUNLinSolFree' was not declared in this scope
       SUNLinSolFree(sunLinearSolver);
       ^~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:188:21: error: 'sunJacMatrix' was not declared in this scope
       SUNMatDestroy(sunJacMatrix);
                     ^~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:188:21: note: suggested alternative: 'DiagMatrix'
       SUNMatDestroy(sunJacMatrix);
                     ^~~~~~~~~~~~
                     DiagMatrix
../libinterp/dldfcn/__ode15__.cc:188:7: error: 'SUNMatDestroy' was not declared in this scope
       SUNMatDestroy(sunJacMatrix);
       ^~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:188:7: note: suggested alternative: 'N_VDestroy'
       SUNMatDestroy(sunJacMatrix);
       ^~~~~~~~~~~~~
       N_VDestroy
../libinterp/dldfcn/__ode15__.cc: In member function 'void octave::IDA::set_up(const ColumnVector&)':
../libinterp/dldfcn/__ode15__.cc:418:9: error: 'sunJacMatrix' was not declared in this scope
         sunJacMatrix = SUNDenseMatrix (num, num);
         ^~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:418:9: note: suggested alternative: 'DiagMatrix'
         sunJacMatrix = SUNDenseMatrix (num, num);
         ^~~~~~~~~~~~
         DiagMatrix
../libinterp/dldfcn/__ode15__.cc:418:24: error: 'SUNDenseMatrix' was not declared in this scope
         sunJacMatrix = SUNDenseMatrix (num, num);
                        ^~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:418:24: note: suggested alternative: 'SparseMatrix'
         sunJacMatrix = SUNDenseMatrix (num, num);
                        ^~~~~~~~~~~~~~
                        SparseMatrix
../libinterp/dldfcn/__ode15__.cc:422:9: error: 'sunLinearSolver' was not declared in this scope
         sunLinearSolver = SUNLinSol_Dense (yy, sunJacMatrix);
         ^~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:422:27: error: 'SUNLinSol_Dense' was not declared in this scope
         sunLinearSolver = SUNLinSol_Dense (yy, sunJacMatrix);
                           ^~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:426:13: error: 'IDASetLinearSolver' was not declared in this scope
         if (IDASetLinearSolver (mem, sunLinearSolver, sunJacMatrix))
             ^~~~~~~~~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:426:13: note: suggested alternative: 'IDAGetLastOrder'
         if (IDASetLinearSolver (mem, sunLinearSolver, sunJacMatrix))
             ^~~~~~~~~~~~~~~~~~
             IDAGetLastOrder
../libinterp/dldfcn/__ode15__.cc:429:24: error: 'IDASetJacFn' was not declared in this scope
         if (havejac && IDASetJacFn (mem, IDA::jacdense) != 0)
                        ^~~~~~~~~~~
../libinterp/dldfcn/__ode15__.cc:429:24: note: suggested alternative: 'IDASetId'
         if (havejac && IDASetJacFn (mem, IDA::jacdense) != 0)
                        ^~~~~~~~~~~
                        IDASetId
../libinterp/dldfcn/__ode15__.cc: At global scope:
../libinterp/dldfcn/__ode15__.cc:437:52: error: 'SUNMatrix' has not been declared
                       N_Vector& yy, N_Vector& yyp, SUNMatrix& JJ)
                                                    ^~~~~~~~~
../libinterp/dldfcn/__ode15__.cc: In member function 'void octave::IDA::jacdense_impl(realtype, realtype, _generic_N_Vector*&, _generic_N_Vector*&, int&)':
../libinterp/dldfcn/__ode15__.cc:457:7: error: 'SUNDenseMatrix_Data' was not declared in this scope
       SUNDenseMatrix_Data(JJ));
       ^~~~~~~~~~~~~~~~~~~


Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #46653:  sunklu.diff added by mtmiller (3KiB - 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 rik5 (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by cdf (Posted a comment)
  • -email is unavailable- added by mtmiller
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-05 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2019-04-02 mtmiller StatusIn Progress Ready For Test
    2019-03-28 mtmiller Attached File- Added sunklu.diff, #46653
    2019-03-28 mtmiller StatusReady For Test In Progress
    2019-03-27 rik5 Carbon-CopyRemoved 72865 -
    2019-03-27 mtmiller StatusIn Progress Ready For Test
    2019-03-27 mtmiller StatusConfirmed In Progress
        Assigned toNone mtmiller
    2019-03-18 mtmiller Carbon-Copy- Added cdf
    2019-03-16 mtmiller StatusNone Confirmed
    2019-03-16 mtmiller Carbon-CopyRemoved 80942 -

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code