bugGNU Octave - Bugs: bug #53489, linspace and logspace with...

 
 

bug #53489: linspace and logspace with endpoints (Inf,Inf) is different than Matlab

Submitter:  Dildar Sk <hodor123456>
Submitted:  Tue 27 Mar 2018 06:16:24 PM UTC
   
 
Category:  Octave Function Severity:  2 - Minor
Priority:  3 - Low Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Dildar Sk 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

Mon 21 Jan 2019 08:04:17 PM UTC, comment #15: 

Excellent; thanks.

Andrew Janke <apjanke>
Mon 21 Jan 2019 04:14:50 PM UTC, comment #14: 

Bug #55538

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Mon 21 Jan 2019 08:19:08 AM UTC, comment #13: 

Please file a new bug report about this test failure using llvm libc++ library, this bug is resolved. There are a handful of other known bugs using the llvm C++ runtime (for example bug #47413, bug #52627).

Mike Miller <mtmiller>
Group Member
Mon 21 Jan 2019 07:50:59 AM UTC, comment #12: 

I compiled octave on linux with clang and libc++ and I get the same error:


octave:1> test logspace
***** assert (logspace (Inf + 1i, Inf + 1i, 3), repmat (complex (-Inf,Inf), [1, 3]))
!!!!! test failed
ASSERT errors for:  assert (logspace (Inf + 1i, Inf + 1i, 3),repmat (complex (-Inf, Inf), [1, 3]))

  Location  |  Observed  |  Expected  |  Reason
    (1)        Inf+NaNi    -Inf+Infi     'NaN' mismatch
    (2)        Inf+NaNi    -Inf+Infi     'NaN' mismatch
    (3)        Inf+NaNi    -Inf+Infi     'NaN' mismatch


This is with clang 7.0.1 and libc++ 7.0.0

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Mon 21 Jan 2019 07:27:19 AM UTC, comment #11: 

I suspect this might be the problem with llvm's
libc++ (that, as far as I know, MacOS is using).

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Mon 21 Jan 2019 07:24:48 AM UTC, comment #10: 

FWIW, on Windows 7:

>> test logspace
PASSES 16 out of 16 tests
>> 10 ^ (Inf+1i)
ans = -Inf + Infi


with tip of yesterday (2ae2dcf0f4d5 dump_demos.m: Use viridis colormap by default.)

Philip Nienhuis <philipnienhuis>
Group Member
Mon 21 Jan 2019 02:35:15 AM UTC, comment #9: 

Is this bug actually fixed, at least on macOS? The test is still failing for me in Octave default (hg commit 2ae2dcf0f4d5) on macOS 10.13.6.


octave:2> test logspace
***** assert (logspace (Inf + 1i, Inf + 1i, 3), repmat (complex (-Inf,Inf), [1, 3]))
!!!!! test failed
ASSERT errors for:  assert (logspace (Inf + 1i, Inf + 1i, 3),repmat (complex (-Inf, Inf), [1, 3]))

  Location  |  Observed  |  Expected  |  Reason
    (1)        Inf+NaNi    -Inf+Infi     'NaN' mismatch
    (2)        Inf+NaNi    -Inf+Infi     'NaN' mismatch
    (3)        Inf+NaNi    -Inf+Infi     'NaN' mismatch
octave:3> 10 ^ (Inf+1i)
ans =  Inf - NaNi
octave:4> ver
----------------------------------------------------------------------
GNU Octave Version: 6.0.0 (hg id: unknown)
GNU Octave License: GNU General Public License
Operating System: Darwin 17.7.0 Darwin Kernel Version 17.7.0: Fri Nov  2 20:43:16 PDT 2018; root:xnu-4570.71.17~1/RELEASE_X86_64 x86_64
----------------------------------------------------------------------
Package Name   | Version | Installation directory
---------------+---------+-----------------------
generate_html  |   0.3.1 | /Users/janke/octave/generate_html-0.3.1
           io  |  2.4.12 | /Users/janke/octave/io-2.4.12


I don't think the bug is in linspace itself here, but in the "10 ^ linspace(...)" operation that logspace is doing.

Andrew Janke <apjanke>
Thu 12 Apr 2018 09:50:43 PM UTC, comment #8: 

I merged the two patches and committed it as one changeset on the development branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/ca022a8c4015).

I modified the commit message to be a bit more explicit about what was changed in the code.


* CMatrix.cc (ComplexMatrix::linspace): Change delta calculation to yield 0 if endpoints are the same, otherwise calculate delta as normal.


Thank you Maor.  Marking as fixed and closing report.

Rik <rik5>
Group administrator
Sat 07 Apr 2018 07:01:57 AM UTC, comment #7: 

Added tests in logspace.m containing Inf endpoints. The test are the same as in linspace.

Maor Shutman <maors>
Fri 06 Apr 2018 09:12:16 PM UTC, comment #6: 

Thank you, this patch and commit message look perfect.

I do think we want some equivalent logspace tests. It's a good habit to test all functions, even if you know that one is a simple wrapper around another. If someone wants to rewrite logspace later, or makes a change or a typo somewhere, it's good to have exhaustive tests that make no assumptions about how functions are written.

Mike Miller <mtmiller>
Group Member
Fri 06 Apr 2018 05:34:55 PM UTC, comment #5: 

I have Matlab and all the tests match exactly.

Regarding logspace - as it calls linspace I don't think that additional tests are needed, but if you want them for symmetry I'll add them.

Fixed mentioned code and commit style problems.

Maor Shutman <maors>
Thu 05 Apr 2018 01:16:41 AM UTC, comment #4: 

Maor - you don't need to open a patch tracker item to submit a patch for a bug, attaching it here is good enough as you have done. Even better would be to post a comment saying that you have attached a fix, because otherwise its existence is hidden way at the bottom of the page here.

The change you have written looks functionally correct and was exactly along the lines of what I was thinking as a fix for this, good job. This looks like a clean fix and addresses all of the typed linspace functions.

The test cases look right to me, do you happen to have access to Matlab to see whether these all behave the same? Do you care to add a few more test cases, like maybe [0,∞], [-∞,0], reverse ranges like [∞,-∞]? Should there be test cases added to logspace.m as well?

The rest of this feedback is about the style rather than the content of your patch.

Please make sure your coding style matches the Octave style guide (https://wiki.octave.org/Octave_style_guide), the test cases you have added so far don't.

Can you change your hg username to be your full name with proper capitalization?

The commit message should be written to follow the commit message guidelines (https://wiki.octave.org/Commit_message_guidelines). The format of your commit message looks correct, but not the message itself. The summary line should describe the change, not just "fix bug #foo". Something like "linspace: handle ranges with equal Inf endpoints (bug #53489)".

The commit message body needs to mention every file and function within that file that was touched. See https://hg.savannah.gnu.org/hgweb/octave/rev/a3bd04422a87 for a perfect example of a commit message that changed the same method in the various Matrix classes.

Mike Miller <mtmiller>
Group Member
Wed 28 Mar 2018 01:25:37 AM UTC, comment #3: 

I'm not going to work on this patch further at this point, but I think this would be a pretty straightforward change to make and would be a good bug for a new contributor to work on to get familiar with liboctave and so on.

Mike Miller <mtmiller>
Group Member
Tue 27 Mar 2018 10:45:14 PM UTC, comment #2: 

I wonder if it might be more useful to have a special case to handle equal endpoints that may be ±∞. Something like this change


--- a/liboctave/array/dRowVector.cc
+++ b/liboctave/array/dRowVector.cc
@@ -275,7 +275,7 @@ linspace (double x1, double x2, octave_i

   retval(0) = x1;

-  double delta = (x2 - x1) / (n - 1);
+  double delta = (x1 == x2) ? 0 : ((x2 - x1) / (n - 1));
   for (octave_idx_type i = 1; i < n-1; i++)
     retval(i) = x1 + i*delta;


and applied to all of the linspace implementations.

Mike Miller <mtmiller>
Group Member
Tue 27 Mar 2018 06:35:00 PM UTC, comment #1: 

This is an odd corner case.  I wonder if it might be more useful to return all NaNs.

Rik <rik5>
Group administrator
Tue 27 Mar 2018 06:16:24 PM UTC, original submission:  

In MATLAB,


>> logspace(inf,inf)

ans =

  Columns 1 through 18

   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf

  Columns 19 through 36

   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf

  Columns 37 through 50

   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf   Inf

>>


But in Octave,


>> logspace(inf,inf)
ans =

 Columns 1 through 11:

   Inf   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN

 Columns 12 through 22:

   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN

 Columns 23 through 33:

   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN

 Columns 34 through 44:

   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN

 Columns 45 through 50:

   NaN   NaN   NaN   NaN   NaN   Inf

>>


Dildar Sk <hodor123456>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43830:  53489.cset added by maors (1KiB - application/octet-stream - logspace: added test cases for Inf endpoints)
file #43824:  53489.cset added by maors (6KiB - application/octet-stream - Fix for bug #53489.)
file #43759:  53489.cset added by maors (5KiB - application/octet-stream - Fix for bug #53489.)

 

Depends on the following items: None found

Items that depend on this one

Digest:
   patch dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by apjanke (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by maors (Updated the item)
  • -email is unavailable- added by hodor123456 (Submitted the item)
  • -email is unavailable- added by hodor123456
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2019-01-21 mtmiller Carbon-CopyRemoved 80942 -
    2018-04-12 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2018-04-07 maors Attached File- Added 53489.cset, #43830
    2018-04-06 maors Attached File- Added 53489.cset, #43824
    2018-04-05 mtmiller Dependencies- patch #9613 is dependent
    2018-04-05 mtmiller StatusConfirmed Patch Submitted
    2018-04-01 maors Attached File- Added 53489.cset, #43759
    2018-03-28 mtmiller Severity3 - Normal 2 - Minor
        Priority5 - Normal 3 - Low
    2018-03-27 mtmiller Summarylogspace return value for corner case of inputs (Inf,Inf) is different than Matlab linspace and logspace with endpoints (Inf,Inf) is different than Matlab
    2018-03-27 rik5 Carbon-CopyRemoved 72865 -
    2018-03-27 rik5 Item GroupInaccurate Result Matlab Compatibility
        StatusNone Confirmed
        SummaryDifference answer than MATLAB. logspace return value for corner case of inputs (Inf,Inf) is different than Matlab
    2018-03-27 hodor123456 Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code