bugGNU Octave - Bugs: bug #57323, Matlab griddata includes both...

 
 

bug #57323: Matlab griddata includes both Octave's griddata and griddata3. Octave's related griddata errors also misleading

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Fri 29 Nov 2019 03:03:38 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Nicholas Jankowski Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 10 Mar 2020 10:16:36 PM UTC, comment #10: 

I reviewed your patch and added some additional changes to the error message strings and to the BIST tests and checked it all in under your name here: https://hg.savannah.gnu.org/hgweb/octave/rev/37ee7c5cc6b2.

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Tue 18 Feb 2020 03:39:11 PM UTC, comment #9: 

I think it's fine to leave it as griddata3().  When griddata3 is marked as deprecated then we can go through the entire code base at once and remove all calls to the function.

Rik <rik5>
Group administrator
Tue 18 Feb 2020 05:05:54 AM UTC, comment #8: 

ok, thanks. i see your note in the griddata3 bug about it being deprecated. I could probably edit the griddata patch so it skips griddata3 altogether and just calls griddatan directly if that would be preferred.

Nicholas Jankowski <nrjank>
Group Member
Tue 18 Feb 2020 03:59:04 AM UTC, comment #7: 

Marking as "Patch Submitted".  I'll see if I can review this in the next couple of days.  It will now go on the development branch as the 6.1 release is closed except for bug fixes.

Rik <rik5>
Group administrator
Mon 17 Feb 2020 02:46:24 AM UTC, comment #6: 

updating patch so it can merge with latest changes, and included a matlab compatibility note in NEWS.

separated out the griddata3 bug since it's an actual implementation error with a small easy fix. filed with patch as bug #57835

this bugfix should be marked as dependent on bug #57835.  last test will fail without griddata3 being patched as it will ignore method 'nearest'.

Note, that matlab doesn't have a griddata3, and since griddata3 just passes data to griddatan, griddata3 could be entirely cut out and deprecated. for now though, it's still here and compatibility is easiest by just creating a pass-through.





(file #48430, file #48431)

Nicholas Jankowski <nrjank>
Group Member
Fri 29 Nov 2019 06:10:58 AM UTC, comment #5: 

ok, patch attached.  griddata3 now respects the method input (had to change the expected value for one test) and griddata can take 3d input and pass it to griddata 3.  updated docstring to show this change, and added the griddata3 tests called through griddata. all tests pass.

(file #47946)

Nicholas Jankowski <nrjank>
Group Member
Fri 29 Nov 2019 04:20:29 AM UTC, comment #4: 

griddata3 method error is an easy fix since it just passes it directly to griddatan. added the default "method='linear'" to the input, and added method to the function call for griddatan and everything works fine whether a method is specified or not for griddata3.  looking to modify griddata to wrap griddata3, will rollup this fix into that patch unless it would be preferred to have them handled separately.


Nicholas Jankowski <nrjank>
Group Member
Fri 29 Nov 2019 04:06:26 AM UTC, comment #3: 

reading the code it also appears that the current version of griddata3 completely ignores the 'method' input.

Nicholas Jankowski <nrjank>
Group Member
Fri 29 Nov 2019 03:22:58 AM UTC, comment #2: 

just to complete the compatibility discussion, matlab includes the following interpolation methods:

for matlab's griddata: nearest, linear, natural, cubic (2d only), v4 ("matlab v4 compatible griddata method, 2d only).  "all except v4 are based on a Delaunay triangulation"  v4 apparently had some smoothing function, rather than defaulting to 'linear'. griddatan only has linear and nearest.

it appears Octave only has nearest and linear implemented in griddata, griddata3, and griddatan.

Nicholas Jankowski <nrjank>
Group Member
Fri 29 Nov 2019 03:06:27 AM UTC, comment #1: 

nevermind about the error message. I am just now realizing that griddata read the v vector for the xi input and the error message was accurate.

Nicholas Jankowski <nrjank>
Group Member
Fri 29 Nov 2019 03:03:38 AM UTC, original submission:  

while trying to run an example from the Matlab help, I got an odd error from Octave.

here's the example:


       xyz = -1 + 2*gallery('uniformdata',[5000 3],0);
       x = xyz(:,1); y = xyz(:,2); z = xyz(:,3);
       v = x.^2 + y.^2 + z.^2;
       d = -0.8:0.05:0.8;
       [xq,yq,zq] = meshgrid(d,d,0);
       vq = griddata(x,y,z,v,xq,yq,zq);
       surf(xq,yq,vq);


it should produce the image in the attached examplefig.png file.

The error received from octave running this same example was: 

>> vq = griddata(x,y,z,v,xq,yq,zq);
error: griddata: XI and YI must be vectors or matrices of same size
error: called from
    griddata at line 76 column 5


an interesting and unhelpful error because:

>> size(xq), size(yq)
ans =

   33   33

ans =

   33   33


I then noticed that Octave' griddata only takes:
ZI = griddata (X, Y, Z, XI, YI, METHOD)

and has a griddata3 function that takes:
VI = griddata3 (X, Y, Z, V, XI, YI, ZI, METHOD, OPTIONS)

according to the matlab help for griddata, it accepts optionally accepts the inputs taken by griddata3, except it does not recognize 'options' to be passed to the internal triangulation function.

seems compatibility could maybe be achieved by extending griddata with a wrapper to call griddata three depending on input count/type.  Without stepping through it, I'm not sure what triggered the misleading error message regarding xi yi sizes not matching.

(note that the example runs without error on Octave by changing the griddata call to griddata3)


Nicholas Jankowski <nrjank>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #48430:  griddata_compatibiltyfix.diff added by nrjank (9KiB - application/octet-stream - updated patch - only for griddata+NEWS. not griddata3)
file #48431:  griddata.m added by nrjank (8KiB - text/plain - updated patch - only for griddata+NEWS. not griddata3)
file #47946:  griddata_fixes.diff added by nrjank (11KiB - application/octet-stream)
file #47943:  examplefig.png added by nrjank (69KiB - image/png)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2020-03-10 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2020-03-10 rik5 Dependencies- Depends on bugs #57835
    2020-02-18 rik5 StatusNone Patch Submitted
        Release5.1.0 dev
        Operating SystemMicrosoft Windows Any
    2020-02-17 nrjank Attached File- Added griddata_compatibiltyfix.diff, #48430
        Attached File- Added griddata.m, #48431
    2019-11-29 nrjank Attached File- Added griddata_fixes.diff, #47946
    2019-11-29 nrjank Attached File- Added examplefig.png, #47943

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code