bugGNU Octave - Bugs: bug #65927, movfun: output shape incorrect for...

 
 

bug #65927: movfun: output shape incorrect for dim>2

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Fri 28 Jun 2024 01:38:56 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Ready For Test Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * 9.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  9.3.0 (current stable)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 05 Jul 2024 06:57:45 PM UTC, comment #10: 

noting that the bug #65211 issue was fixed and re-closed. This report staying as Ready for Test for now.

Nicholas Jankowski <nrjank>
Group Member
Mon 01 Jul 2024 02:01:18 AM UTC, comment #9: 

reopened bug #65221 to address the issue.

Nicholas Jankowski <nrjank>
Group Member
Sun 30 Jun 2024 05:14:57 PM UTC, comment #8: 

See also bug #65221 that motivated that commit afaict.

Markus Mützel <mmuetzel>
Group administrator
Sun 30 Jun 2024 05:10:46 PM UTC, comment #7: 

IIUC, the new test uncovered an issue that is present since a few months. I guess it is caused by this change (that is still on the default branch only):
https://hg.savannah.gnu.org/hgweb/octave/rev/4df9460ab8a9

One of the input parser validation functions needs to be able to catch the value of `x` of its scope:

    parser.addParamValue ("dim", [], ...
      @(d) isempty (d) || (isscalar (d) && isindex (d, ndims (x))));


However, the anonymous function is only initialized once on the first call of "movfun". Subsequent calls (with potentially differently shaped `x`) re-use the inputParser object from before.
That causes the error that is uncovered by the new test.

CC'ing arb as the author of that change.

@arb: Should we revert that change? Is it possible to update the validation function of an existing inputParser object?
I guess that change was done for performance reasons. How bad was it before that change?

Markus Mützel <mmuetzel>
Group administrator
Sun 30 Jun 2024 09:20:13 AM UTC, comment #6: 

One of the tests is failing on the default branch for the buildbots. E.g.:
https://buildbot.octave.org/#/builders/23/builds/1107/steps/7/logs/stdio

>>>>> processing /home/buildbotu/fc25-x86_64/gcc-lto-fedora/src/scripts/signal/movfun.m
***** 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)))"
shared variables
    x =
        1    2    3    4    5    6    7    8    9   10
    wlen =
       2   1
    wlen02 =
       0   2
    wlen20 =
       2   0
    ctrfun =
@(x) x (wlen (1) + 1, :)
    UNO =
       1
       1
       1
       1
       1
       1
       1


Markus Mützel <mmuetzel>
Group administrator
Fri 28 Jun 2024 07:32:53 PM UTC, comment #5: 

patch passes make check. pushed to stable as

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

marking as ready for test.

regarding the dims>ndims issue (along with some others), created bug #65928 to track that. 

Nicholas Jankowski <nrjank>
Group Member
Fri 28 Jun 2024 04:03:05 PM UTC, comment #4: 

also noted that movfun throws an error for dim>ndim, which is a matlab incompatibility. unless i find another report addressing it, will file a new one.

Nicholas Jankowski <nrjank>
Group Member
Fri 28 Jun 2024 03:34:09 PM UTC, comment #3: 

it appears that the shape restore just needs the permute changed to ipermute.  still passes all tests, i think it only seems to matter for dim>2 and there don't seem to be any nd tests.  I'll work up a patch with some tests.

Nicholas Jankowski <nrjank>
Group Member
Fri 28 Jun 2024 03:00:28 PM UTC, comment #2: 

can someone familiar with movfun comment a bit on the 'Format 1 / Format 2' output forms and whether that may be involved in this bug? I don't see any indication of it being a matlab compatabiltiy feature, but I don't quite follow from the help text what the discriminating factors are.

Nicholas Jankowski <nrjank>
Group Member
Fri 28 Jun 2024 01:42:12 PM UTC, comment #1: 

for completeness - the expected output for dim 3 is:


>> movmean(a,2,3)

ans(:,:,1) =

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


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


Nicholas Jankowski <nrjank>
Group Member
Fri 28 Jun 2024 01:38:56 PM UTC, original submission:  

As pointed out over on Stackoverflow, it appears that movfun fails to return correct dimensions for dim>2.  see below:


>> b = reshape (1:30, 5, 3, 2)
b =

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

> movfun(@mean, b, 2, 'dim', 1)
ans =

ans(:,:,1) =

    1.0000    6.0000   11.0000
    1.5000    6.5000   11.5000
    2.5000    7.5000   12.5000
    3.5000    8.5000   13.5000
    4.5000    9.5000   14.5000

ans(:,:,2) =

   16.000   21.000   26.000
   16.500   21.500   26.500
   17.500   22.500   27.500
   18.500   23.500   28.500
   19.500   24.500   29.500

>> movfun(@mean, b, 2, 'dim', 2)
ans =

ans(:,:,1) =

    1.0000    3.5000    8.5000
    2.0000    4.5000    9.5000
    3.0000    5.5000   10.5000
    4.0000    6.5000   11.5000
    5.0000    7.5000   12.5000

ans(:,:,2) =

   16.000   18.500   23.500
   17.000   19.500   24.500
   18.000   20.500   25.500
   19.000   21.500   26.500
   20.000   22.500   27.500

>> movfun(@mean, b, 2, 'dim', 3)
ans =

ans(:,:,1) =

    1.0000    8.5000
    6.0000   13.5000
   11.0000   18.5000

ans(:,:,2) =

    2.0000    9.5000
    7.0000   14.5000
   12.0000   19.5000

ans(:,:,3) =

    3.0000   10.5000
    8.0000   15.5000
   13.0000   20.5000

ans(:,:,4) =

    4.0000   11.5000
    9.0000   16.5000
   14.0000   21.5000

ans(:,:,5) =

    5.0000   12.5000
   10.0000   17.5000
   15.0000   22.5000


I suspect it's something to do with the permute/ipermute sequence  that forces all operations along dim1, for simpler computation, but haven't had a chance to investigate yet.

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 mmuetzel
  • -email is unavailable- added by mmuetzel (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-06-30 mmuetzel Carbon-Copy- Added arb
    2024-06-28 nrjank StatusNeed Info Ready For Test
    2024-06-28 nrjank StatusConfirmed Need Info

    Back to the top

    Powered by Savane 3.13-b921.
    Corresponding source code