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 FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Inaccurate Result
Status: Wont FixAssigned to: None
Originator Name: AvinoamOpen/Closed: Closed
Release: devOperating 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 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>
Project Member
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>
Project Member
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>
Project Member
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:

With gcc 5:

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>
Project Member
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:

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>
Project Member
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>
Project Member
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

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.

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>
Project Member
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>
Project Member
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:

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 File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by tmacchant (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by lachlan (Posted a comment)
  • -unavailable- added by avinoam (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
    Sat 10 Sep 2016 10:01:02 AM UTCavinoamDependencies-=>bugs #49036 is dependent
    Sun 08 May 2016 04:13:20 PM UTCmtmillerStatusConfirmed=>Wont Fix
      Open/ClosedOpen=>Closed
    Tue 03 May 2016 06:48:28 PM UTCrik5Summaryrealpow 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
    Sun 01 May 2016 03:07:06 PM UTCmtmillerSummaryrealpow and sparse-xpow.cc-tst fail with gcc 4=>realpow and sparse-xpow.cc-tst fail with C++11 and gcc 4
    Sun 01 May 2016 03:04:51 PM UTCmtmillerSummaryrealpow and sparse-xpow.cc-tst fails=>realpow and sparse-xpow.cc-tst fail with gcc 4
    Sun 01 May 2016 03:00:11 PM UTCmtmillerDependencies-=>bugs #47214 is dependent
    Sun 24 Apr 2016 01:08:01 PM UTClachlanItem GroupIncorrect Result=>Inaccurate Result
      StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1