bugGNU Octave - Bugs: bug #51779, bsxfun unit tests occasionally...

 
 

bug #51779: bsxfun unit tests occasionally fail on certain random inputs

Submitter:  Mike Miller <mtmiller>
Submitted:  Fri 18 Aug 2017 12:37:01 AM UTC
   
 
Category:  Test Suite Severity:  4 - Important
Priority:  5 - Normal Item Group:  Regression
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Open
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

Fri 02 Dec 2022 09:31:38 PM UTC, comment #12: 

Checking in a year later before 8.1 is released. Status from comment #11 is the same: promoting to double works OK as a workaround.

Arun Giridhar <arungiridhar>
Group Member
Mon 29 Nov 2021 09:13:03 PM UTC, comment #11: 

Checking in three years later before 7.1 is released. The status is exactly as before:


>> bsxfun (@power, y, x) == power (y, x)
ans =

  0  1
  1  1

>> bsxfun (@power, double(y), double(x)) == power (double(y), double(x))
ans =

  1  1
  1  1

>> bsxfun (@power, double(y), x) == power (double(y), x)
ans =

  1  1
  1  1

>> bsxfun (@power, y, double(x)) == power (y, double(x))
ans =

  1  1
  1  1


Promoting x or y or both to double eliminates the error, and Matlab errors on nearly all mixed-type combinations, so it's not a compatibility issue.

Does this cause any test suite errors at present? If not, is this OK to close since there is a ready workaround?

Arun Giridhar <arungiridhar>
Group Member
Mon 31 Dec 2018 08:11:43 PM UTC, comment #10: 

Okay.  I can't see us solving before the 5.0 release so I am taking it off the 5.0 bug fix list.

Rik <rik5>
Group administrator
Mon 31 Dec 2018 06:45:14 PM UTC, comment #9: 

The original example in this bug report still fails with Octave 5.0.1:


>> x = [int32(-5) 0];
>> y = [single(-0.0226); 0];
>> assert (bsxfun (@power, y, x), power (y, x))
error: ASSERT errors for:  assert (bsxfun (@power, y, x),power (y, x))

  Location  |  Observed  |  Expected  |  Reason
   (1,1)      -169612453   -169612448    Abs err 5 exceeds tol 0 by 5


So the original issue is still present, that bsxfun treats operands differently from unary ops with the same arguments.

Mike Miller <mtmiller>
Group Member
Mon 31 Dec 2018 05:01:07 PM UTC, comment #8: 

Do we want to do anything further on this report or close it?

Rik <rik5>
Group administrator
Thu 24 Aug 2017 04:00:12 PM UTC, comment #7: 

Sorry to be late to this.  In reference to comment #5, I believe we have historically demoted rather than promoted to double.  For example,


single (1) + 2
=>
single (1) + single (2)


The alternative would be to promote to double and then cast back to single at the end.


single (1) + 2
=>
single (double (single (1)) + double (2))


But I don't think we do that.

So if possible, this is the desired computation


int32 (single (x) ^ single (int32 (y)))



Rik <rik5>
Group administrator
Thu 24 Aug 2017 02:24:56 PM UTC, comment #6: 

maybe not news to anyone, but was curious about matlab compatibility. turns out it's a non issue. Matlab errors out on almost all of these:


>> x = [int32(-5) 0];
y = [single(-0.0226); 0];
assert (bsxfun (@power, y, x), power (y, x))
Error using bsxfun
Mixed integer class inputs are not supported.

>> x = [ 2.6, -4.8, 0.3; -2.6, -1.6, -3.4; 1.2, 4.1, 4.9];
y = [-0.0226; 3.6882; -2.3801];
assert (bsxfun (@power, single (y), int32 (x)), power (single (y), int32 (x)))
Error using bsxfun
Mixed integer class inputs are not supported.

>> power (single ([-0.0226; 0]), int32 ([-5 0]))
Error using  .^
Integers can only be combined with integers of the same class, or
scalar doubles.

>> single ([-0.0226; 0]) .^ int32 ([-5 0])
Error using  .^
Integers can only be combined with integers of the same class, or
scalar doubles.


Nicholas Jankowski <nrjank>
Group Member
Fri 18 Aug 2017 05:04:22 AM UTC, comment #5: 

I made the following change to fix this


diff --git a/liboctave/util/oct-inttypes.cc b/liboctave/util/oct-inttypes.cc
--- a/liboctave/util/oct-inttypes.cc
+++ b/liboctave/util/oct-inttypes.cc
@@ -753,7 +753,7 @@ pow (const octave_int<T>& a, const doubl
 template <typename T>
 octave_int<T>
 pow (const float& a, const octave_int<T>& b)
-{ return octave_int<T> (std::pow (a, b.float_value ())); }
+{ return octave_int<T> (std::pow (static_cast<double> (a), b.double_value ())); }

 template <typename T>
 octave_int<T>


Basically, pow(float,octave_int<T>) used to call pow(double,double), and was unintentionally changed at this point to call powf(float,float).

It's not immediately obvious which one of these is "correct", but it is empirically obvious that the new code calling powf is giving a different answer than what Octave normally gives for the scalar operation of single(x)^int32(y).

I think this may have been discussed on another report, but should a mixed operation like single(x)^int32(y) be the same as int32(double(x)^double(int32(y)))? Or should it be the same as int32(single(x)^single(int32(y)))?

Mike Miller <mtmiller>
Group Member
Fri 18 Aug 2017 04:03:25 AM UTC, comment #4: 

This regression was introduced with

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

Use modf, pow from C++ std library. These functions are guaranteed to be part of C++11 standard library.

Something about this has changed the following two operations, compared to Octave 4.2.1:


power (single ([-0.0226; 0]), int32 ([-5 0]))
single ([-0.0226; 0]) .^ int32 ([-5 0])


This has something to do with the mixed types and the broadcasting expansion. This does not affect the normal elementwise exponentiation when both operands have the same dimension. And calling bsxfun explicitly is also not affected.

This is possibly because the change is now calling std::pow, which is a function template, so different underlying C functions might be called. Before this change, these few spots in the code were calling the C function pow, which always takes its arguments as doubles.

I may try forcing some of these std::pow calls to operate on doubles to see if anything has an effect by trial and error.

Adding Rik in cc.

Mike Miller <mtmiller>
Group Member
Fri 18 Aug 2017 01:51:49 AM UTC, comment #3: 

Oops. Operator problem. I guess one needs to reset randn each time.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 18 Aug 2017 01:47:58 AM UTC, comment #2: 

I see the problem in reduced example, but not in the one with load randon_state (PASSES 76 out of 76 tests).

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 18 Aug 2017 12:39:27 AM UTC, comment #1: 

Just tested this in Octave 4.2.1 and I see that the same expression does not fail, so recategorizing as a regression on the default branch.

Mike Miller <mtmiller>
Group Member
Fri 18 Aug 2017 12:37:01 AM UTC, original submission:  

I see intermittent test failures in the test scripts in libinterp/corefcn/bsxfun.cc. At least one case that I have captured and reproduced is due to a call to power with a particularly bad set of arguments.

A much reduced minimal example that shows the problem operation first:


x = [int32(-5) 0];
y = [single(-0.0226); 0];
assert (bsxfun (@power, y, x), power (y, x))


This is a reduced and simplified version of the following test case that is actually pulled from the test scripts in bsxfun.cc:


x = [ 2.6, -4.8, 0.3; -2.6, -1.6, -3.4; 1.2, 4.1, 4.9];
y = [-0.0226; 3.6882; -2.3801];
assert (bsxfun (@power, single (y), int32 (x)), power (single (y), int32 (x)))


And I pulled this one data set from an actual test run, as can be shown by loading the attached file and running the test suite as follows:


load random_states;
rand ("state", rand_state);
test libinterp/corefcn/bsxfun.cc


I can easily pick some arbitrary random state to hardcode into the test scripts to ensure that this doesn't happen, but maybe someone wants to look at this further to see if something actually does need fixing, or whether the random vectors can be constrained better.

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 #41564:  random_states added by mtmiller (15KiB - application/octet-stream)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by mtmiller
  • -email is unavailable- added by dasergatskov (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-12-31 mtmiller Carbon-CopyRemoved 80942 -
    2018-04-13 mtmiller Dependencies- bugs #53636 is dependent
    2017-08-18 mtmiller StatusNone Confirmed
        Carbon-Copy- Added rik5
    2017-08-18 mtmiller Severity3 - Normal 4 - Important
        Item GroupInaccurate Result Regression
    2017-08-18 mtmiller Attached File- Added random_states, #41564

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code