bugGNU Octave - Bugs: bug #65928, movfun dimension constraints:...

 
 

bug #65928: movfun dimension constraints: shouldn't error for dim > ndims(x), or wlen > size(x, dim), or wlen=1

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Fri 28 Jun 2024 06:01:57 PM UTC
   
 
Category:  Octave Function Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 9.2.0 Operating System:  * Any
Fixed Release:  10.1.0 (current default) Planned Release:  10.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 25 Oct 2024 09:35:03 PM UTC, comment #10: 

Changesets look good.

Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Fri 26 Jul 2024 06:20:01 PM UTC, comment #9: 

bug #66025 created to cover implementing the other options.

Nicholas Jankowski <nrjank>
Group Member
Fri 26 Jul 2024 12:37:00 AM UTC, comment #8: 

pushed https://hg.savannah.gnu.org/hgweb/octave/rev/4ce02ce746c0
to default. allows dim>ndims(x), adjusts some of the subfunctions to handle different window lengths, adds more BISTs

I think that satisfies everything in this report.  still some separate feature requests for samplepoints and nanflags options. those can be handled in a separate feature request report.

marking as ready for test.

Nicholas Jankowski <nrjank>
Group Member
Mon 22 Jul 2024 07:33:27 PM UTC, comment #7: 

pushed the fix for allowing wlen = 1 or [0,0] as
https://hg.savannah.gnu.org/hgweb/octave/rev/4526e97589f2

Nicholas Jankowski <nrjank>
Group Member
Mon 22 Jul 2024 12:26:03 AM UTC, comment #6: 

pushed a couple changes to default:

1)
https://hg.savannah.gnu.org/hgweb/octave/rev/d2d5fbc6934b
allows all movfun to accept non-integer valued wlen.  not one of the summary issues, but required for some of the later changes.

2)
https://hg.savannah.gnu.org/hgweb/octave/rev/e5e41c222908
allows wlen to take values larger than size(x, dim). there were a number of errors with different wlen inputs.  e.g., something like movfun(@sum, 1:10, 11) would throw an error, while movfun(@sum, 1:10, [5 6]) would pass input validation but trigger other unhandled errors later.

this should also allow wlen = 1, and dim > ndims(x) (which was necessarily hitting wlen > size(x, dim) since higher dims are singleton). 

also expect once these are tackled, enabling the Samplepoints option should also be a straightforward add.

Nicholas Jankowski <nrjank>
Group Member
Fri 05 Jul 2024 04:55:22 PM UTC, comment #5: 

some of these issues lie in movslice.  removed a fixme for basic functionality BISTs for movslice to stable as
https://hg.savannah.gnu.org/hgweb/octave/rev/f23f7db9a37b

(and made sure it doesn't break default this time.)  expect to have a patch for some wlen features to default soon.

Nicholas Jankowski <nrjank>
Group Member
Mon 01 Jul 2024 06:02:26 PM UTC, comment #4: 

sorry, that errors if dim >= ndims(x)

Nicholas Jankowski <nrjank>
Group Member
Mon 01 Jul 2024 06:01:40 PM UTC, comment #3: 

a patch was pushed to default to fix the issue causing the test failure.  probably worth noting that correcting this bug would remove the problematic code that checks for dim to be >= ndims(x).

Nicholas Jankowski <nrjank>
Group Member
Mon 01 Jul 2024 01:42:03 AM UTC, comment #2: 

those were tests for bug #65927.  it passes for me on stable, I'm rebuilding default now to test.  I think markus found bug #65221 which makes inputParser persistent in movfun on default. I'm not sure how that is creating the error within the test environment though. that error is what i would expect if calling

movfun (@mean, a 2, 'dim', 4) 

(calling for dim4 operation on a dim3 object). guessing it's something about the persistent parser change. I'm guessing it may be possible to sidestep the test failure by splitting that into two %!test blocks, but it shouldn't be necessary.

Nicholas Jankowski <nrjank>
Group Member
Sat 29 Jun 2024 09:16:44 PM UTC, comment #1: 

I guess the tests need to be adjusted:

***** test <*65927>
 a = reshape (1:30, 5, 3, 2);
 b1 = cat (3, [1, 6, 11], [16, 21, 26]) + [0, 0.5, 1.5, 2.5, 3.5]';
 b2 = cat (3, [1:5]', [16:20]') + [0, 2.5, 7.5];
 b3 = cat (3, [1:5]', [8.5:1:12.5]') + [0, 5, 10];
 assert (movfun (@mean, a, 2), b1, eps);
 assert (movfun (@mean, a, 2, 'dim', 1), b1, eps);
 assert (movfun (@mean, a, 2, 'dim', 2), b2, eps);
 assert (movfun (@mean, a, 2, 'dim', 3), b3, eps);

 a2 = cat (4, a, a, a, a);
 assert (size (movfun (@mean, a2, 2)), [5, 3, 2, 4]);
 assert (size (movfun (@mean, a2, 2, 'dim', 1)), [5, 3, 2, 4])
 assert (size (movfun (@mean, a2, 2, 'dim', 2)), [5, 3, 2, 4]);
 assert (size (movfun (@mean, a2, 2, 'dim', 3)), [5, 3, 2, 4]);
 assert (size (movfun (@mean, a2, 2, 'dim', 4)), [5, 3, 2, 4]);
!!!!! regression: https://octave.org/testfailure/?65927
movfun: failed validation of 'dim'.  Checked with "@(d) isempty (d) || (isscalar (d) && isindex (d, ndims (x)))"


Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 28 Jun 2024 06:01:57 PM UTC, original submission:  

The moving window functions calling movfun currently throw an error if the requested operating dimension is larger than the ndims(x).  If that is relaxed, then an error occurs because wlen > 1 (and it errors separately for wlen < 2).

all of those are matlab incompatible.  relaxing the dim <=ndims is fairly simple, but the rest may be rather involved.

here are a handful of things that should produce valid outputs, as per matlab 2024a:


x = reshape (1:30, 5, 3, 2);

>> movmean(x,2,4)

ans(:,:,1) =

     1     6    11
     2     7    12
     3     8    13
     4     9    14
     5    10    15


ans(:,:,2) =

    16    21    26
    17    22    27
    18    23    28
    19    24    29
    20    25    30

>> movmean(x,29,3)

ans(:,:,1) =

    8.5000   13.5000   18.5000
    9.5000   14.5000   19.5000
   10.5000   15.5000   20.5000
   11.5000   16.5000   21.5000
   12.5000   17.5000   22.5000


ans(:,:,2) =

    8.5000   13.5000   18.5000
    9.5000   14.5000   19.5000
   10.5000   15.5000   20.5000
   11.5000   16.5000   21.5000
   12.5000   17.5000   22.5000

>> movmean(x,29,4)

ans(:,:,1) =

     1     6    11
     2     7    12
     3     8    13
     4     9    14
     5    10    15


ans(:,:,2) =

    16    21    26
    17    22    27
    18    23    28
    19    24    29
    20    25    30

>> movmean(x,1,2)

ans(:,:,1) =

     1     6    11
     2     7    12
     3     8    13
     4     9    14
     5    10    15


ans(:,:,2) =

    16    21    26
    17    22    27
    18    23    28
    19    24    29
    20    25    30

>> movmean(x,4,2)

ans(:,:,1) =

    3.5000    6.0000    6.0000
    4.5000    7.0000    7.0000
    5.5000    8.0000    8.0000
    6.5000    9.0000    9.0000
    7.5000   10.0000   10.0000


ans(:,:,2) =

   18.5000   21.0000   21.0000
   19.5000   22.0000   22.0000
   20.5000   23.0000   23.0000
   21.5000   24.0000   24.0000
   22.5000   25.0000   25.0000

>> movmean(x,5,2)

ans(:,:,1) =

     6     6     6
     7     7     7
     8     8     8
     9     9     9
    10    10    10


ans(:,:,2) =

    21    21    21
    22    22    22
    23    23    23
    24    24    24
    25    25    25


>> movmean(x,1,5)

ans(:,:,1) =

     1     6    11
     2     7    12
     3     8    13
     4     9    14
     5    10    15


ans(:,:,2) =

    16    21    26
    17    22    27
    18    23    28
    19    24    29
    20    25    30


Nicholas Jankowski <nrjank>
Group Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

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 mmuetzel (Updated the item)
  • -email is unavailable- added by dasergatskov (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-10-25 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 10.1.0 (current default)
    2024-07-26 nrjank StatusIn Progress Ready For Test
        Planned ReleaseNone 10.1.0 (current default)
    2024-07-05 nrjank StatusNone In Progress
    2024-06-30 mmuetzel Summarymovfun dimension contstraints: shouldn't error for dim &gt; ndims(x), or wlen &gt; size(x, dim), or wlen=1 movfun dimension constraints: shouldn't error for dim > ndims(x), or wlen > size(x, dim), or wlen=1

    Back to the top

    Powered by Savane 3.14-49f2.
    Corresponding source code