bugGNU Octave - Bugs: bug #47775, realpow and sparse-xpow.cc-tst...

 
 

bug #47775: realpow and sparse-xpow.cc-tst fail with C++11 and gcc < 4.9.2

Submitted by:  Avinoam Kalma <avinoam>
Submitted on:  Sun 24 Apr 2016 11:00:58 AM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Inaccurate Result
Status:  Wont Fix Assigned to:  None
Originator Name:  Avinoam Open/Closed:  Closed
Release:  dev Operating System:  GNU/Linux

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)

Tue 19 Jul 2016 03:40:42 AM UTC, comment #28: 

I have installed gcc 5.3.0 using ubuntu ppa  and built again. FAILs are disappeared.

I will use gcc 5.3.0 on octave 4.1.0+ or later.

Tatsuro MATSUOKA <tmacchant>
Mon 18 Jul 2016 09:36:21 AM UTC, comment #27: 

I have met the same phenomena on Ubuntu 14.04 amd64 (gcc 4.8.4).
As written, do I have to upgrade gcc?

Tatsuro MATSUOKA <tmacchant>
Thu 26 May 2016 09:08:57 PM UTC, comment #26: 

I had the realpow/sparse-xpow issue with gcc 4.7.3 on RHEL.
I upgraded to 4.9.3, and can confirm this fixed the issue.

Anonymous
Sun 08 May 2016 04:13:20 PM UTC, comment #25: 

Alright, closing as won't fix.

Mike Miller <mtmiller>
Project Administrator
Tue 03 May 2016 10:59:43 PM UTC, comment #24: 

I agree with Rik.

If either input to realpow(i,2) is changed by an epsilon, then realpow should fail.  If code relies on it succeeding, that is like code relying on (sin(pi) == 0) == true, or 0:0.1:10 having exactly 11 elements.

All of these are discrete magnifications of (lack of) round-off error.  Why should  exp (y * log (x)) should give real results for even y and imaginary x, unless (sin((y/2)*pi) == 0) == true?

It seems that the libraries "normally" take integer y as a special case and implement it by repeated multiplication and g++11 4.8 didn't, but unless it is documented that it should take that special case, it is not a bug for it not to.

Sorry if I'm just adding noise to this discussion, but I haven't heard an argument why this round-off is different from any other round-off affecting edge cases.

Lachlan Andrew <lachlan>
Tue 03 May 2016 07:13:48 PM UTC, comment #23: 

The bigger error is realpow(i,2) failing.

Mike Miller <mtmiller>
Project Administrator
Tue 03 May 2016 07:09:41 PM UTC, comment #22: 

If it is only some tests failing failing by a few eps then I don't think it is worth all the testing in configure.ac.

Rik <rik5>
Project Administrator
Tue 03 May 2016 07:02:25 PM UTC, comment #21: 

I think it's clearly a gcc bug that has been fixed.

My only reason for leaving open is, do we want to allow for older gcc to be used by either adding a --without-c++11 option, or test for defects like this and automatically disable C++11 support? As I said in comment #15, we could add more configure detection and handle this, but that would also add more maintenance cost and more code conditionals.

When we looked at adding C++11, we didn't want to immediately drop all old compilers, but bugs like this start to erode support for stable/old versions more quickly.

Mike Miller <mtmiller>
Project Administrator
Tue 03 May 2016 06:48:28 PM UTC, comment #20: 

Can we close this report?  It seems that this is a gcc bug already fixed upstream, rather than a bug that Octave needs to fix.

Rik <rik5>
Project Administrator
Tue 03 May 2016 04:27:33 PM UTC, comment #19: 

The correct output is shown with gcc 4.9.2 but not gcc 4.8.2 (the version I tested in comment #15).

This means that mxe-octave already uses a version of gcc where this is fixed, so it should not appear in the Windows builds.

Mike Miller <mtmiller>
Project Administrator
Tue 03 May 2016 09:57:20 AM UTC, comment #18: 

That looks like a great tool.  Do you know what standard gcc 4.9.2 defaults to?  Is it C++11 or C++03?  The problem only seems to appear using C++11.

Lachlan Andrew <lachlan>
Tue 03 May 2016 08:44:00 AM UTC, comment #17: 

I tested on-line here with gcc 4.9.2

https://www.codechef.com/ide

that reports (-1, 0) too.

Anonymous
Mon 02 May 2016 12:09:49 AM UTC, comment #16: 

A simple response would be to make realpow give a once-off run time warning if it was built with a version of GCC before 5.0, suggesting upgrading to 5.0 if the user expects integer exponents to be computed exactly.

Except for the Matrix case, the special case code for integers seems just to call a different library routine, rather than doing something that would ensure a real answer.  This would be fixed if the scalar case also did the repeated multiplication when the GCC version is less than 5.

Soon I'll try to write a patch to show what I mean.

Lachlan Andrew <lachlan>
Sun 01 May 2016 02:58:15 PM UTC, comment #15: 

Octave already has special code in the xpow functions for integer exponents, the point is that old versions of gcc is not doing the right thing, and how much do we care about that when it's already been fixed in gcc 5.

With gcc 4.8:

$ cat cplxtest.cc
#include <complex>
#include <iostream>

int main (int argc, char *argv[])
{
  std::complex<double> x (0, 1);
  int y = 2;

  std::cout << std::pow (x, y) << std::endl;
}
$ g++ cplxtest.cc -o cplxtest
$ ./cplxtest
(-1,0)
$ g++ -std=gnu++11 cplxtest.cc -o cplxtest
$ ./cplxtest
(-1,1.22465e-16)

With gcc 5:

$ g++ cplxtest.cc -o cplxtest
$ ./cplxtest
(-1,0)
$ g++ -std=gnu++11 cplxtest.cc -o cplxtest
$ ./cplxtest
(-1,0)

This is clearly a bug in gcc through version 4.8, I understand that this breaks the realpow function, not just a failing unit test, so I'm not sure what we should do about this. We could add more configure tests, and disable the C++11 flags automatically on compilers that have this bug. This would add more maintenance cost with conditionals for things like std::unique_ptr vs std::auto_ptr.

Mike Miller <mtmiller>
Project Administrator
Sun 01 May 2016 11:59:33 AM UTC, comment #14: 

Thanks, Mike.

With those settings, the stable branch also gives the errors in realpow and sparse-xpow.

What is the next step?  Should we write special code for integer exponents?

Is there hope that this is just a glitch in the library that will be fixed in the next version? Is it documented that integer exponents are supposed to give exact results in these cases?  What about -1^(1/2)? -1^(1/3)?

Lachlan Andrew <lachlan>
Thu 28 Apr 2016 06:48:51 PM UTC, comment #13: 

This sounded familiar and did come up before, see bug #46065.

I forgot both C and C++ compilers need to agree, and gcc 4.8 did not default to C11 like gcc 5 does. Try with CC="gcc -std=gnu11" and CXX="g++ -std=gnu++11".

Mike Miller <mtmiller>
Project Administrator
Thu 28 Apr 2016 03:29:56 AM UTC, comment #12: 

Mike Miller knows a lot about the different compilers.  He is on this bug report and might know whether the -std=gnu++11 works with older libc versions.

Rik <rik5>
Project Administrator
Tue 26 Apr 2016 10:04:03 AM UTC, comment #11: 

When I try to configure and make with "CXX="g++ -std=gnu++11", I get a compile error:

In file included from ../libgnu/time.h:42:0,
                 from /usr/include/pthread.h:24,
                 from /usr/include/x86_64-linux-gnu/c++/4.8/bits/gthr-default.h:35,
                 from /usr/include/x86_64-linux-gnu/c++/4.8/bits/gthr.h:148,
                 from /usr/include/c++/4.8/ext/atomicity.h:35,
                 from /usr/include/c++/4.8/bits/ios_base.h:39,
                 from /usr/include/c++/4.8/ios:42,
                 from /usr/include/c++/4.8/istream:38,
                 from /usr/include/c++/4.8/sstream:38,
                 from /usr/include/c++/4.8/complex:45,
                 from ../../liboctave/util/oct-cmplx.h:27,
                 from ../../liboctave/array/Array-C.cc:30:
../libgnu/stddef.h:93:3: error: conflicting declaration 'typedef union max_align_t max_align_t'
} max_align_t;

Any suggestions?

I'm completely happy with keeping the tight tolerances.

Oh, and I shouldn't have referred to [3,4] regarding tolerance. Those were for the sparse-xpow test, but the same holds for an epsilon change in the exponent (or base) used in the realpow test.  The point is that (IMHO) we shouldn't rely on floating point libraries to distinguish between "integer" and "non-integer"; they're "entitled" to assume all floating point values are approximations, and to return approximations as results -- unless they are documented to do otherwise.

Rather than blacklisting particular versions of tools or libraries, I think we should write the integer-power case explicitly if the tolerance is unacceptable.  It is compact and relatively fast in principle (2log_2(exponent) multiplications).

Lachlan Andrew <lachlan>
Tue 26 Apr 2016 05:22:16 AM UTC, comment #10: 

OK. I'll try to make time this evening.

Note however that although the difference between real and complex is huge if the input is real, it is not huge if the input is already complex, which is the case here.

If the [3,4] in the test were replaced by [3+eps, 4+eps], then the true result would indeed be complex.  If we want to treat integers as "special", then there is a case for doing that ourselves, rather than relying on the complex library to do it.  That is what I had to do in pown() in oct-stream.cc to get 1e+10 etc. to be exact integers in textscan.

Lachlan Andrew <lachlan>
Mon 25 Apr 2016 03:22:34 PM UTC, comment #9: 

I am very reluctant to increase the tolerance on BIST tests when they have always worked in the past.  Historically, the very strict tolerances have led us to find real bugs which would have been ignored if we had just defaulted to relaxing the tolerances rather than finding the true problem.

In this case, the difference in realpow is very significant and not just a tolerance issue.  A complex result is very, very different from a real result.

If we need to blacklist a compiler, or use different options, then that is what we need to discover.  But most compilers haven't had a problem.

I like Mike's suggestion to test with the -std option.  It doesn't seem that there is any reason to run the full make check either.  I believe just

./configure --necessary_options
make
run-octave -f
test realpow

should be enough.

Rik <rik5>
Project Administrator
Mon 25 Apr 2016 11:41:22 AM UTC, comment #8: 

It is not just the realpow test that fails,
the realpow function will throw an error.

>> octave_config_info.hg_id
ans = da9c75ffd1a3

>> realpow(i,2)
error: realpow: produced complex result
error: called from
    realpow at line 36 column 5
stopped in /../octave/scripts/specfun/realpow.m at line 36
36:     error ("realpow: produced complex result");

Anonymous
Mon 25 Apr 2016 12:13:13 AM UTC, comment #7: 

I think someone should test with gcc 4.8 to see if the presence or absence of the "-std=gnu++11" option causes the test to pass or fail. I think this could most easily be done on the stable branch, simply by building stable with CXX="g++ -std=gnu++11" vs the default of CXX=g++.

If the tests do fail with that option added, then this is the same as bug #47214. That's the first question to answer.

I can't say whether we care about exact answers, all I can say is that at a glance this looks like a test that someone intended to be exact, and is exact with recent versions of the gcc compiler. Maybe some library defect in older versions of gcc causes the wrong call to be evaluated somewhere, as gcc C++11 compliance was still being worked on, and has since been fixed.

Mike Miller <mtmiller>
Project Administrator
Mon 25 Apr 2016 12:01:57 AM UTC, comment #6: 

Mike, I'm not sure if it answers your question in comment #4, but I am using gcc 4.8.2 and get the test failure on a recent snapshot.

I haven't checked whether it started when C++11 was enabled.

To me, the question is "does it matter?"  It is just roundoff.  Should we care that taking integer powers of integers is inexact?  If so, we can fix it.  If not, we can allow slack in the tests.

Am I missing something?

Lachlan Andrew <lachlan>
Sun 24 Apr 2016 09:22:15 PM UTC, comment #5: 

Er, anonymous.

Mike Miller <mtmiller>
Project Administrator
Sun 24 Apr 2016 09:21:58 PM UTC, comment #4: 

Good cross-reference Lachlan. Bug #47214 was with gcc 4.6. Can we say definitively that these test failures also occur with gcc 4.8 (the default compiler on Ubuntu 14.04) since the move to enabling C++11 by default?

Mike Miller <mtmiller>
Project Administrator
Sun 24 Apr 2016 06:31:43 PM UTC, comment #3: 

realpow and sparse_xpow issues also mentioned in http://savannah.gnu.org/bugs/?47214

Anonymous
Sun 24 Apr 2016 06:08:49 PM UTC, comment #2: 

Can you use 'hg bisect' to locate where this problem first appeared?

Rik <rik5>
Project Administrator
Sun 24 Apr 2016 01:08:01 PM UTC, comment #1: 

Yes, I have noticed that too, on LMDE.

It is easy to make the test pass by putting in a tolerance, but it isn't clear if that is the right approach.

For small real integer powers, should we use repeated multiplication instead of exp(log(.) * .)?  For powers up to say 1000, it would only take about 10 multiplications and so may be faster as well as avoiding this sort of round-off.

Lachlan Andrew <lachlan>
Sun 24 Apr 2016 11:00:58 AM UTC, original submission:  

Running __run_test_suite__ on Ubuntu 14.04
with changes set 6459479840ba I got 2 failures:

>>>>> processing /usr/local/share/octave/4.1.0+/m/specfun/realpow.m
***** assert (realpow (1i,2), -1)
!!!!! test failed
realpow: produced complex result

>>>>> processing /usr/local/share/octave/4.1.0+/etc/tests/libinterp/corefcn/sparse-xpow.cc-tst
***** assert (sparse (2i) .^ [3, 4], sparse ([-0-8i, 16]))
!!!!! test failed
ASSERT errors for:  assert (sparse (2i) .^ [3, 4],sparse ([-0 - 8i, 16]))

  Location  |  Observed  |  Expected  |  Reason
    (1)      -1.46958e-15-8i     0-8i       Abs err 2.3054e-15 exceeds tol 0
    (2)      16-3.91887e-15i    16+0i       Abs err 4.3027e-15 exceeds tol 0

I did not get these failures on Windows with MXE-Octave.

Avinoam Kalma <avinoam>
Project 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

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by tmacchant (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by avinoam (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-09-10 avinoam Dependencies- => bugs #49036 is dependent
    2016-05-08 mtmiller StatusConfirmed => Wont Fix
        Open/ClosedOpen => Closed
    2016-05-03 rik5 Summaryrealpow and sparse-xpow.cc-tst fail with C++11 and gcc 4 => realpow and sparse-xpow.cc-tst fail with C++11 and gcc < 4.9.2
    2016-05-01 mtmiller Summaryrealpow and sparse-xpow.cc-tst fail with gcc 4 => realpow and sparse-xpow.cc-tst fail with C++11 and gcc 4
    2016-05-01 mtmiller Summaryrealpow and sparse-xpow.cc-tst fails => realpow and sparse-xpow.cc-tst fail with gcc 4
    2016-05-01 mtmiller Dependencies- => bugs #47214 is dependent
    2016-04-24 lachlan Item GroupIncorrect Result => Inaccurate Result
        StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.5