bugGNU Octave - Bugs: bug #66256, movmad uses 'mean absolute...

 
 

bug #66256: movmad uses 'mean absolute deviation', matlab uses 'median absolute deviation'

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Thu 26 Sep 2024 01:30:25 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  nrjank
Originator Name:  Open/Closed:  * Closed
Release:  * 10.0.90 Release: 
Operating System:  * Any Fixed Release:  10.1.0
Planned Release:  10.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 07 Apr 2025 09:49:59 PM UTC, comment #10: 

No reports of any problem in a month.  I think it is safe to change this bug report to Fixed and close it.

Rik <rik5>
Group administrator
Tue 11 Mar 2025 03:54:48 AM UTC, comment #9: 
Nicholas Jankowski <nrjank>
Group Member
Tue 11 Mar 2025 03:54:03 AM UTC, comment #8: 

pushed attached patch to stable renaming variable and updating doctrings. marking as ready for test.

Nicholas Jankowski <nrjank>
Group Member
Fri 07 Mar 2025 07:54:09 PM UTC, comment #7: 

temporarily opening and updating tags for considering release candidate fix

Nicholas Jankowski <nrjank>
Group Member
Fri 07 Mar 2025 03:46:16 PM UTC, comment #6: 

attached is a proposed docstring/variable renaming patch that changes the name of the option from 'mode' to 'method', as I wasn't thinking when I chose the former that it's the name of the third general used central tendency measurement of statistical datasets, and shouldn't probably be the name of the option we use to chose one of the other two central tendency methods.

build/test machine isn't available at the moment, it passes test movmad within the octave 10 RC, but i haven't verified build with the change.

attached for consideration before 10 final release.  not sure if something more 'unique' than method might be better to further reduce liklihood of matlab feature collision  (central_method, center_method?, central_tendency?)

(file #56979)

Nicholas Jankowski <nrjank>
Group Member
Tue 22 Oct 2024 12:15:38 AM UTC, comment #5: 

This looks nice.  I re-wrapped the Texinfo documentation to 80 characters (https://hg.savannah.gnu.org/hgweb/octave/rev/4e01707840b9).

Marking bug as Fixed and closing report.

Rik <rik5>
Group administrator
Sat 19 Oct 2024 01:31:20 PM UTC, comment #4: 

ok, changed the parameter to a name/value pair and updated the descriptions/news/tests here:

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

remarking Ready for Test

Nicholas Jankowski <nrjank>
Group Member
Sat 19 Oct 2024 02:48:50 AM UTC, comment #3: 

a good point.  I was originally thinking of copying the wonderfully nonintuitive OPT = 0 or 1 approach used by mad, but that would collide with dim inputs so i went with one keyword. just didn't occur to me to pull parse_moveargs out of movfun for processing. i'll work up a version like that and see how it looks.

Nicholas Jankowski <nrjank>
Group Member
Fri 18 Oct 2024 08:47:07 PM UTC, comment #2: 

I took a look at the changeset.  I had a philosophical question about how the option was implemented.  Instead of adding a new text option "MODE" which creates a different API than Matlab, would it be better to use a property NAME/VALUE pair?  Matlab has this interface already, and they don't use the property name "mode", so it would be easy to overload.  The function _parse_moveargs_ will simply pass these options through so you could take a look at the output of that for the keyword "mode".

Rik <rik5>
Group administrator
Fri 27 Sep 2024 01:41:57 AM UTC, comment #1: 

pushed https://hg.savannah.gnu.org/hgweb/octave/rev/803e6fc006ed to default.  patch changes the default from mean to median.  it adds an optional mode input that allows the user to select median or mean. the option is stripped from varargin before passing the call to movfun, with fcn being either "@mad" or "@(x) mad (x, 1)". adds tests, doc notes, news note.

So this makes the code compatible, ands an extension to let octave continue calling a moving mean absolute deviation which matlab doesn't have. it's been 8 years, but if they add it later, this method of enabling the selection should be adaptable to whatever they later do.

marking as ready for test.

Nicholas Jankowski <nrjank>
Group Member
Thu 26 Sep 2024 01:30:25 AM UTC, original submission:  

while both matlab and octave's mad functions default to mean abs deviation and have an option to do median abs deviation,  the movfun versions do not have the option.  Just noticed when verifying other movfun bists that matlab uses median, while octave's was built to use mean.   working on a patch to switch to median, but might also add a flag to allow the user to choose. if it's too cumbersome, will just note the change in the news and help and direct people wanting the old incompatible behavior to use movfun directly.

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 #56979:  movmad_changemodetomethod.patch added by nrjank (9KiB - application/octet-stream - patch to rename new option from mode to method)

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2025-04-07 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2025-03-11 nrjank StatusNeed Info Ready For Test
    2025-03-07 nrjank StatusFixed Need Info
        Open/ClosedClosed Open
        Release9.2.0 10.0.90
    2025-03-07 nrjank Attached File- Added movmad_changemodetomethod.patch, #56979
    2024-10-22 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 10.1.0
    2024-10-19 nrjank StatusIn Progress Ready For Test
    2024-10-19 nrjank StatusReady For Test In Progress
    2024-09-27 nrjank StatusIn Progress Ready For Test

    Back to the top

    Powered by Savane 3.15-64aa.
    Corresponding source code