bugGNU Octave - Bugs: bug #50583, Empty arguments to some simple...

 
 

bug #50583: Empty arguments to some simple functions give inconsistent results to Matlab

Submitter:  Ceral Paquet <octavebugs>
Submitted:  Sat 18 Mar 2017 09:25:07 PM UTC
   
 
Category:  Octave Function Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.2.1 Operating System:  * Any
Fixed Release:  9.1.0 Planned Release:  9.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 09 Jul 2023 10:51:58 PM UTC, comment #16: 

corrected corr handling of row vectors, verified matlab compatibility, and added BISTs to cover missing case.  see bug #64395 for any further discussion.

Nicholas Jankowski <nrjank>
Group Member
Sat 08 Jul 2023 10:15:59 PM UTC, comment #15: 

Will review the last change and see what I missed. As noted in the other report, could be related to corr or cov

Nicholas Jankowski <nrjank>
Group Member
Sat 08 Jul 2023 08:39:09 PM UTC, comment #14: 

The corr() now broken in a new way:


octave:1> a=[1,2,3]; b = a; corr(a,b)
error: c(3,_): out of bound 1 (dimensions are 1x1)
error: called from
    corr at line 86 column 7
octave:2> version -hgid
ans = e67b7b85670b
octave:3>


See bug
https://savannah.gnu.org/bugs/index.php?64395

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 12 Apr 2023 09:00:46 PM UTC, comment #13: 

No complaints in a few weeks.  Marking bug as Fixed and closing report.

Rik <rik5>
Group administrator
Thu 23 Mar 2023 03:19:29 AM UTC, comment #12: 

made very minor changes to corr and corrcoef to account for the new return size from cov. spearman passed once corr was fixed. those tests all pass. patch applied passes a make check against current default, so hopefully no more surprises.

note that there may be packages calling cov that will have to make similar changes to what was made in corr and corrcoef. this was detailed in an updated NEWS entry for the cov change.

re-pushed as
https://hg.savannah.gnu.org/hgweb/octave/rev/e67b7b85670b
marking as Ready for Test again.  this time if there are no issues with this and mode pushed with comment #5, everything covered by this report should be fixed.

Nicholas Jankowski <nrjank>
Group Member
Tue 21 Mar 2023 03:21:27 AM UTC, comment #11: 

ok. backed out 01ae7ebc6a13.  corr was built expecting that calling form of cov. will sort that out and check for any other conflicts. 

backed out cov patch attached here for the time being, reverting status to In Progress.

(file #54517)

Nicholas Jankowski <nrjank>
Group Member
Tue 21 Mar 2023 02:43:33 AM UTC, comment #10: 

as mentioned over in bug #48690,  corr calls cov using it's old cov(x,y) form which is causing BIST failures in corr, corrcoeff, and spearman. 

looking now to see if it's a quick patch or if this needs to be backed out. the 'if you want the old form you can use the statistics package' fix doesn't quite work for core functions.

Nicholas Jankowski <nrjank>
Group Member
Tue 21 Mar 2023 12:46:36 AM UTC, comment #9: 

updated cov refreshed (actually significantly rewritten) from bug #48690 version.  handles empty and nan inputs, nanflag options, and is now matlab compatible in handling two inputs arrays. added quite a few BISTs.

pushed to default as https://hg.savannah.gnu.org/hgweb/octave/rev/01ae7ebc6a13 along with an update to the NEWS.9.md file. Marking as ready for test.

If no issues with this or the version of mode pushed with comment #5, this should complete fixing everything covered by this bug report.

Nicholas Jankowski <nrjank>
Group Member
Sat 04 Mar 2023 06:34:10 PM UTC, comment #8: 

needs to be 'refreshed' (I wrote it 6-7 years ago). but I'll put it as my next to do

Nicholas Jankowski <nrjank>
Group Member
Sat 04 Mar 2023 03:14:27 PM UTC, comment #7: 

No objections from me.  No functionality is being lost (it exists either in cov or covm) and the code itself is written so there is no development hurdle.  It feels like we should just do it.

Rik <rik5>
Group administrator
Sat 04 Mar 2023 06:13:24 AM UTC, comment #6: 

question about cov:  as was discussed over on bug #48690, cov is deliberately matlab incompatible for two input arrays.  This makes producing a compatible empty input response a bit more difficult because only part of the input options can be tested against matlab. additionally, there was an inconsistency in how cov interpreted scalar inputs that further complicated things.  

It had been decided (back in 2017) to go ahead and make a fully compatible version of cov, with a warning in the help, and a note that the old behavior could be obtained by using the NaN package's covm function.

I wrote that version of cov to fix bug #48690 (and can still be found attached there), which was then pushed to the statistics package .  Looking at this again with an intent make the core version of cov compatible with empty inputs, and having a nanflag option, is there any strong objection to pushing such a version to core for Octave 9 with the same notes/warnings about backward compatibility?

Nicholas Jankowski <nrjank>
Group Member
Sat 04 Mar 2023 06:03:24 AM UTC, comment #5: 

Revived the old changes to mode.m to have compatible empty handling and applied them against the current version.  Also improved the dim input validation to cover empty, Inf, NaN, etc, and added a bunch of BISTs. As this is more a compatibility concern than an error-causing bug, I added a note to News9 and pushed to default as:

https://hg.savannah.gnu.org/hgweb/octave/rev/96ed8d1d0c59

Nicholas Jankowski <nrjank>
Group Member
Wed 01 Mar 2023 05:06:28 PM UTC, comment #4: 

patch #10314 was pushed to default (ported from statistics to core) that includes matlab compatible mean, median, var, and std handling of empty inputs.  it also ensures compatible size of empty output for those functions. 

mode and cov still return incompatible outputs for empty inputs. statistics appears to no longer shadow those two functions that contained the bug #48690 patch to fix this. While I'm sure there are other functions that also have this compatibility issue, those two remain the only ones mentioned in this report that are not yet addressed in octave core.


Nicholas Jankowski <nrjank>
Group Member
Wed 20 Nov 2019 11:26:40 AM UTC, comment #3: 

I came across the same issue whereby median([]) returns an error in Octave while it returns NaN in Matlab. It seems that it was fixed with bug #48690 in the statistics toolbox but scripts/statistics/median.m in Octave is still unmodified.

Guillaume <gyom>
Wed 28 Jun 2017 06:20:44 PM UTC, comment #2: 

note that this is related to bug #48690 which has patches submitted for producing fully matlab compatible output from basic statistics functions std, var, median, mean, mode, and cov.

Nicholas Jankowski <nrjank>
Group Member
Sun 19 Mar 2017 05:22:16 AM UTC, comment #1: 

This is a philosophical difference.  Octave has generally preferred to raise an error early to alert the programmer that something is wrong.  The alternative, which Matlab often does, is to keep computing but use the value NaN.  The NaN value cascades through calculations so at the end of a long script the programmer may find that the value is NaN, but have no idea which step in the process created the problem.  I wouldn't change Octave's behavior in this case, rather I would make sure that mode and cov also issue errors.  But we'll see if other core programmers have an opinion.

Rik <rik5>
Group administrator
Sat 18 Mar 2017 09:25:07 PM UTC, original submission:  

MATLAB 2016a:

>> median([])

ans =

   NaN

>> std([])

ans =

   NaN

>> var([])

ans =

   NaN

>> mode([])
Warning: MODE of a 0-by-0 matrix is NaN; result was an empty matrix in previous releases.
> In mode (line 72)

ans =

   NaN

>> cov([])

ans =

   NaN



Octave 4.2.1:

>> median([])
error: median: X cannot be an empty matrix
error: called from
    median at line 67 column 5
>> std([])
ans = [](0x0)
>> var([])
error: var: X must not be empty
error: called from
    var at line 103 column 5
>> mode([])
ans = [](1x0)
>> cov([])
ans = [](0x0)


Ceral Paquet <octavebugs>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54517:  cov_bug50583.patch added by nrjank (23KiB - application/octet-stream - matlab compatible cov patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by gyom (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by octavebugs (Submitted the item)
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2023-04-12 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0
    2023-03-23 nrjank StatusIn Progress Ready For Test
    2023-03-21 nrjank Attached File- Added cov_bug50583.patch, #54517
        StatusReady For Test In Progress
    2023-03-21 nrjank StatusNeed Info Ready For Test
        Planned ReleaseNone 9.1.0
    2023-03-04 nrjank StatusConfirmed Need Info
    2023-03-04 nrjank StatusNone Confirmed
    2023-03-01 nrjank Operating SystemGNU/Linux Any
    2017-03-19 rik5 CategoryNone Octave Function
        Severity3 - Normal 2 - Minor

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code