bugGNU Octave - Bugs: bug #52475, ode15 is not compatible with...

 
 

bug #52475: ode15 is not compatible with SUNDIALS >= 3.0.0

Submitted by:  Sebastian <sschoeps>
Submitted on:  Tue 21 Nov 2017 09:48:12 PM UTC  
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  In Progress Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Any

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

( Jump to the original submission)

Fri 09 Nov 2018 12:16:03 PM UTC, comment #38:

Your comment #37 seems to be the best choice. Since I
use Fedora I have needed 3.0 for quite a while. But,
right now I am working on other stuff, so it is not
an urgent requirement.

In any case, thanks for all your help.

Michael Godfrey <godfrey>
Project Member
Fri 09 Nov 2018 12:08:58 PM UTC, comment #37:

I plan to keep compatibility with both and will try to extend it also to the coming Sundials 4.0 but will only have time to work on this on December.

I'm really going to use what I proposed in (4) for the reasons I explained there.

Andre da Costa Barros <andrecbarros>
Fri 09 Nov 2018 09:02:04 AM UTC, comment #36:

Question on this: do you want to preserve compatibility with both SUNDIALS 2.7 and 3.x, or just switch to 3.x?

Andrew Janke <apjanke>
Mon 05 Nov 2018 10:24:16 AM UTC, comment #35:

sorry I misspelled a sentence in the previous comment :

"With the patch modified by Orion the SUNDIALS is completely skipped so SUNDIALS becomes a mandatory dependency."

should read

"With the patch modified by Orion the variables set by configure to signal whetehr SUNDIALS is usable are completely ignored so SUNDIALS becomes a mandatory dependency."

to make things more clear I personally I think over-linking would be a minor issue (but I don't know what JWE would think about it).

The real issue is that the pre-processor fags used in conditionals inside _ode15_.cc should be set by configure by testing that a working SUNDIALS is available, not removed and ignored.

Carlo de Falco <cdf>
Project Member
Mon 05 Nov 2018 10:12:26 AM UTC, comment #34:

Andre,

I'm not saying you should assign flags to make the build complete on your system.

What I say is that the configure script should be somehow able to detect whether or not a SUNDIALS installation is present on your system and whether or not it has the correct features, then decide whether or not Octave should link or not to SUNDIALS.

With the patch modified by Orion the SUNDIALS is completely skipped so SUNDIALS becomes a mandatory dependency.

c.

Carlo de Falco <cdf>
Project Member
Mon 05 Nov 2018 09:49:56 AM UTC, comment #33:

From what I gather from configure.ac, indeed, octave does (1).

In my system, qhull doesn't have a .pc file, fftw does (what makes things easier).

The only penalty on giving assignments to *FLAGS to ./configure in step/phase, this is what I'm doing, is related to overlinking and it affects the binaries mostly if we are using static libs, with dynamic libs the unused references would be eliminated with --as-needed option on linking phase (it is not a clear cut, though, e.g., https://wiki.mageia.org/en/Overlinking_issues_in_packaging).

Have (4) implemented is doable to deal with complex cases but I would have to clear the dust from my previous knowledge about autotools. I can do it, but it will take time and I'm in a very busy period right now.

Andre da Costa Barros <andrecbarros>
Mon 05 Nov 2018 01:56:19 AM UTC, comment #32:

Andre,

removing conditionals in the source code means
that building Octave will just outright fail when sundials
is not available or not the correct version.

As sundials is an optional dependency that should not be the case.

If you know some autotools scripting and would like to
propose a solution according to item 4 in your comment #30
that would be indeed welcome, but just removing all
conditionals and forcing the link is not an acceptable option.

To see how other optional dependencies are treated you may
look for example at what is done for FFTW, GLPK or QHULL.

c.

Carlo de Falco <cdf>
Project Member
Sun 04 Nov 2018 08:53:06 PM UTC, comment #31:

Mistakes:
"are not on cached/standard" -> "are not inside cached/standard"
"in may case" -> "in my case"

Sorry for that.

Andre da Costa Barros <andrecbarros>
Sun 04 Nov 2018 07:51:05 PM UTC, comment #30:

Hum, by parts:

1 - I don't know how octave handle it, but on many packages when you set *FLAGS it means a start point, not a whole substitution (more or less equivalent to SOMEFLAGS="$SOMEFLAGS --what-you-set-explicitly";
2 - It would be easier if every package generated a pkg-config .pc file to query where things are located and how to "link" and "include" things. Many don't, sundials doesn't;
3 - octave could get out of its way to guess where things were located using ldd and ldconfig on linux but things get really complicated when we use libraries that are not on cached/standard dirs;
4 - A possible solutions for this would be if octave had an option to, say, "some-random-lib" in the form of --with-some-random-lib-libs="-L<some_dir> -ldep1 -ldep2 .. -ldepn" in addtition to what it already use (--with-some-random-lib=, --with-some-random-lib-libdir= and --some-random-lib-includedir=);

Of course, the patch can and should be improved but, in may case, octave would, possibly, have to address (3) and (4) as I link sundials to superlu_mt and hypre and the standard setting may fail to get the extra linkage (i.e., the configure.in would have to check for extra libraries dependencies; not worth in my opinion as LDFLAGS and CFLAGS are simple and straightforward).

What I described in (4) would be a nice addition, I think (some packages use this).

Andre da Costa Barros <andrecbarros>
Sun 04 Nov 2018 04:27:23 PM UTC, comment #29:

Orion, Andre,

If I understand correctly what you are doing in file #45334
is remove all Preprocessor conditionals in order to force
using the code for sundials, is that the case?

The whole point in the patch is to detect
automatically the flags to set in order to
link with SUNDIALS so, if you need to set
the flags manually that means the patch is
not working.

As I said in comment #25 this part of the
patch was probably not correct even for SUDIALS 2.

Carlo de Falco <cdf>
Project Member
Sat 03 Nov 2018 06:34:59 AM UTC, comment #28:

Additionally, I had to add to LDFLAGS "-lsundials_ida -lsundials_idas -lsundials_sunlinsolklu -lsundials_fsunlinsolklu" but it is working now with sundials 3.2.1.

Thanks to everyone involved!

Andre da Costa Barros <andrecbarros>
Wed 31 Oct 2018 01:49:11 AM UTC, comment #27:

I've attached my updated version of the patch. I also needed to set CPPFLAGS=-I/usr/include/suitesparse in order for the sundials headers to find klu.h.

(file #45334)

Orion Poplawski <opoplawski>
Sat 27 Oct 2018 09:42:50 PM UTC, comment #26:

Any progress here?

Orion Poplawski <opoplawski>
Wed 09 May 2018 08:29:56 AM UTC, comment #25:

thanks for uploading the configuration files.
From what I see:

1) I found an error in the configure tests that I will fix ASAP
2) I am surprised this worked ever before even for sundials2

Carlo de Falco <cdf>
Project Member
Wed 09 May 2018 05:28:21 AM UTC, comment #24:

Yes, the changeset is relative to the default branch.
Yes, you need to run bootstrat to get a new configure script when configure.ac is patched.

Carlo de Falco <cdf>
Project Member
Wed 09 May 2018 04:21:50 AM UTC, comment #23:

config.h (as fc28_config.h) and config.log (as fc28_config.log)
are attached.

Dmitri.

(file #44130, file #44131)

Dmitri A. Sergatskov <dasergatskov>
Wed 09 May 2018 04:06:21 AM UTC, comment #22:

I was messing with this stuff so I would need to recreate them.
Could you verify that
a) patach to be applied on default (dev) branch, not on stable.
b) since it patches configure.ac I need to re-run bootstrap
prior to configure.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 09 May 2018 03:49:50 AM UTC, comment #21:

Dmitri,

Thanks again for testing, I'm still not sure whether KLU for sparse Jacobian should be enabled or disabled for you ...

Can you please upload config.log and config.h?

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 10:54:16 PM UTC, comment #20:

configure step looked better, but compile failed:

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Tue 08 May 2018 09:23:22 PM UTC, comment #19:

here's a new try at the changeset.

(file #44127)

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 09:17:28 PM UTC, comment #18:

sorry, I actually meant this one:

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 09:16:16 PM UTC, comment #17:

do you have the following in config.h?

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 08:59:33 PM UTC, comment #16:

it is in sunlisol sub-directory:

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Tue 08 May 2018 08:53:51 PM UTC, comment #15:

Dmitri,

Thanks for testing.

Indeed there seems to be something wrong, either
SUNLINSOL_KLU is installed but is not detected
or it is not installed, in which case its header
should not be included.

Which one is the case for you? Does "locate sunlinsol_klu.h"
return something for you?

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 08:48:25 PM UTC, comment #14:

comment #13 was truncated:

(which, I think, is wrong)

and during compile:

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 07:44:14 PM UTC, comment #13:

With current dev snapshot (bc5f225bc578+ tip @) on Fedora 28
(Sundials 3.1.0):

during configure:

Dmitri A. Sergatskov <dasergatskov>
Tue 08 May 2018 05:58:04 PM UTC, comment #12:

here is an updated patch, this works for me on macos 10.13 with a self built sudials 3.1.0

can someone else test the patch and/or look at it (especially the configure tests)?

(file #44125)

Carlo de Falco <cdf>
Project Member
Tue 08 May 2018 03:20:26 PM UTC, comment #11:

Hi,
can you please be more specific about the compiler/linker flags you used to build _ode15_.oct from _ode15_.cc?

Carlo de Falco <cdf>
Project Member
Mon 07 May 2018 05:06:34 PM UTC, comment #10:

I built _ode15_.oct using this version of mingw:

c:\temp\Projects\ode15_ming64>where g++
c:\Program Files\mingw-w64\x86_64-7.2.0-win32-seh-rt_v5-rev1\mingw64\bin\g++.exe

Then, I ran these tests:

>> which ode15s

'ode15s' is a function from the file C:\Octave\OCTAVE~3.0\share\octave\4.4.0\m\ode\ode15s.m

>> test ode15s

PASSES 39 out of 39 tests

>> test ode15i

PASSES 48 out of 48 tests

I could do more testing, if needed.

Bill Greene <billgreene>
Mon 07 May 2018 04:20:17 PM UTC, comment #9:

The attached patch also changes configure tests.

(file #44113)

Carlo de Falco <cdf>
Project Member
Mon 07 May 2018 03:43:23 PM UTC, comment #8:

Hi,

I turned your patch into a proper changeset that applies on the current development branch and made a few style fixes.

How did you build and test the code?

If I apply the changeset and run "configure" the tests for IDADENSE and IDAKLU fail and ode15{i,s} get disabled ...

c.

(file #44112)

Carlo de Falco <cdf>
Project Member
Mon 07 May 2018 12:14:43 PM UTC, comment #7:

I think I patched revision 25054 of _ode15_.cc.

I can see that these patch files are rather problematic with
respect to the version of the file they are created for.
So, as you suggested, I just uploaded the full, modified _ode15_.cc. This version has been built and tested with octave 4.4.0.

(file #44110)

Bill Greene <billgreene>
Mon 07 May 2018 11:07:01 AM UTC, comment #6:

Bill, Thanks for your patch and sorry for the late reply.

The patch does seem to have been greated correctly
but it does not apply to the current development branch,
what exact version of Octave did you start with for creating the patch?

If you don't feel like going through the trouble of creating
a well formed mercurial changeset you can just download the latest version of ode15.cc from the hgweb interface using the following link:

http://hg.savannah.gnu.org/hgweb/octave/raw-file/default/libinterp/dldfcn/__ode15__.cc

and upload the full modified file.

c.

Carlo de Falco <cdf>
Project Member
Mon 16 Apr 2018 12:13:35 PM UTC, comment #5:

I attached a patch file to this bug report to update _ode15_.cc to build with sundials 3.1.

Bill Greene <billgreene>
Tue 03 Apr 2018 12:59:03 AM UTC, comment #4:

I added more tests to configure on the stable branch as a stopgap for this bug for the Octave 4.4 release

https://hg.savannah.gnu.org/hgweb/octave/rev/5dce8aa80652

This will automatically disable building against SUNDIALS 3.0.0 and later. Octave 4.4 will build against version 2.7.0 only. This is no longer a build failure error, so recategorizing and retitling.

Please do comment if there are still build errors that the new configure changes don't take care of automatically.

Leaving this bug open until after the release when someone has time to fix it on the development branch by rewriting the interface to the new IDA API.

Mike Miller <mtmiller>
Project Administrator
Fri 30 Mar 2018 07:05:15 PM UTC, comment #3:

I've built a copy of sundials 3.1.0 and I confirm the error and that the API has changed significantly.

I think we're probably short on time to get a true fix in for 4.4, so it might make sense to add tests to configure for the ida_dense API and conditionally compile out those parts of the code. I can take a look at doing that in the short term.

Mike Miller <mtmiller>
Project Administrator
Fri 30 Mar 2018 06:11:17 AM UTC, comment #2:

FYI -- Fedora 28 (beta is out, final is expected to be released on May 1) comes with sundials 3.1.0

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 21 Mar 2018 11:23:11 PM UTC, comment #1:

It would be really nice if someone familiar with the SUNDIALS interface could take a look at this for Octave 4.4, since distributions will probably start moving to SUNDIALS 3 very soon.

Mike Miller <mtmiller>
Project Administrator
Tue 21 Nov 2017 09:48:12 PM UTC, original submission:

As initially reported in [1]: Sundials has changed the treatment of dense matrices [1], such that "ida_dense.h" which is included by "__ode15__.cc" is gone.

From the manual:

> Added generic SUNMATRIX module with three provided implementations:
> dense, banded and sparse.
> These replicate previous SUNDIALS Dls and Sls matrix structures in a
> single object-oriented API.


[1] http://octave.1599824.n4.nabble.com/Sundials-issues-with-ode15-tt4685656.html
[2] https://computation.llnl.gov/sites/default/files/public/idas_guide.pdf

Sebastian <sschoeps>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #45334:  octave-sundials3.patch added by opoplawski (19KiB - text/x-patch - octave-sundials3.patch)
file #44130:  fc28_config.log added by dasergatskov (2MiB - text/x-log)
file #44131:  fc28_config.h added by dasergatskov (114KiB - text/x-chdr)
file #44127:  open_bXICA7Gt.txt added by cdf (18KiB - text/plain)
file #44125:  open_TK6CSjvd.txt added by cdf (18KiB - text/plain)
file #44113:  open_K7lryXre.txt added by cdf (13KiB - text/plain)
file #44112:  open_tkJXJDPU.txt added by cdf (9KiB - text/plain - changes for default branch)
file #44110:  __ode15__.cc added by billgreene (33KiB - text/plain - modified file to support sundials 3.1)
file #43949:  __ode15__.cc.patch added by None (11KiB - application/octet-stream - Patch file for _ode15_.cc with the required, minimal changes to build with sundials 3.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by apjanke (Posted a comment)
  • -email is unavailable- added by andrecbarros (Posted a comment)
  • -email is unavailable- added by opoplawski (Posted a comment)
  • -email is unavailable- added by cdf (Posted a comment)
  • -email is unavailable- added by billgreene (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by siko1056 (Updated the item)
  • -email is unavailable- added by sschoeps (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 21 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-11-04 mtmiller Carbon-CopyRemoved 80942 => -
    2018-10-31 opoplawski Attached File- => Added octave-sundials3.patch, #45334
    2018-05-09 dasergatskov Attached File- => Added fc28_config.log, #44130
        Attached File- => Added fc28_config.h, #44131
    2018-05-08 cdf Attached File- => Added open_bXICA7Gt.txt, #44127
    2018-05-08 cdf Attached File- => Added open_TK6CSjvd.txt, #44125
    2018-05-07 cdf Attached File- => Added open_K7lryXre.txt, #44113
    2018-05-07 cdf Attached File- => Added open_tkJXJDPU.txt, #44112
        Priority1 - Later => 5 - Normal
        StatusPostponed => In Progress
    2018-05-07 billgreene Attached File- => Added _ode15_.cc, #44110
    2018-04-16 None Attached File- => Added _ode15_.cc.patch, #43949
    2018-04-03 mtmiller CategoryConfiguration and Build System => Libraries
    2018-04-03 mtmiller Severity4 - Important => 3 - Normal
        Priority5 - Normal => 1 - Later
        Item GroupBuild Failure => Other
        StatusConfirmed => Postponed
        Summaryode15 does not compile with Sundials >=3.0 => ode15 is not compatible with SUNDIALS >= 3.0.0
    2018-03-30 mtmiller CategoryOctave Function => Configuration and Build System
    2018-03-21 mtmiller Severity3 - Normal => 4 - Important
    2017-11-29 siko1056 StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.3