bugGNU Octave - Bugs: bug #64525, optimset/optimget issues warning,...

 
 

bug #64525: optimset/optimget issues warning, rather than error, when short name matches multiple possible options

Submitter:  Rik <rik5>
Submitted:  Mon 07 Aug 2023 06:02:57 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:  * dev 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

Thu 21 Sep 2023 07:36:28 PM UTC, comment #15: 

No problems reported in a month. Closing as fixed.

Arun Giridhar <arungiridhar>
Group Member
Wed 09 Aug 2023 08:08:32 PM UTC, comment #14: 

pushed a docstring change to default.  While there may be other issues, I believe that satisfies the concerns here and bug #64516.

http://hg.savannah.gnu.org/hgweb/octave/rev/3df98fee838b

marking as ready for test.

Nicholas Jankowski <nrjank>
Group Member
Wed 09 Aug 2023 04:45:47 PM UTC, comment #13: 

if the current function behaivor is acceptable, the fixes left seem to be just documentation updates. this does overlap with bug #64516, but the following could be useful for the optimset/get help:

1 - optimset should list the current Valid Parameters (as it does), and also mention the fact that the list can be extended by the user or other packages (maybe with a redundant note there stating that the full 'active' parameter list can be obtained from a no argument call to optimset.)

2 - should explain (a) the automatic matching process, (b) the expected behavior when an ambiguity occurs, (c) expected behavior when an unambiguous but unknown parameter is specified, (d) that automatic matching will ignore any non-standard/unknown parameters.

3 - state that optimset/get cannot be used to set/query any ambiguous parameters, they can only be accessed manually using dot notation (is that the correct name?)

4 - a warning in optimset that that the optimization options structure is intended only for manipulation of known parameters by optimset/get, and that unknown parameter creation, creation of ambiguous parameters, and loading/unloading packages that change the known parameter list after creating an optimization options structure can result in unpredictable/undesirable behavior.

Nicholas Jankowski <nrjank>
Group Member
Wed 09 Aug 2023 01:48:28 PM UTC, comment #12: 

said another way:  for both optimset and optimget - unrecognized parameters will only trigger warnings. ambiguous parameters will trigger errors.

Nicholas Jankowski <nrjank>
Group Member
Wed 09 Aug 2023 01:46:15 PM UTC, comment #11: 

the comment #8 change to optimset still allows arbitrary parameters to be set, so long as there is no matching ambiguity with default parameters.  if there is, the change will make that an error. 

I don't think any Matlab parameters have full-name ambiguities with current Octave parameters. Are you aware of any that do? If not, then Octave will still allow you to set those properties using optimset and retrieve them with optimget. 

In the case that you still need to set arbitrary field names, the dot convention still functions with both Octave and Matlab (and it's the only way to work with them in Matlab).

if that doesn't cover your user case concern, please explain further so we can better understand

Nicholas Jankowski <nrjank>
Group Member
Wed 09 Aug 2023 12:22:09 PM UTC, comment #10: 

I don't fully understand what is being changed/discussed here. 

In the past, one of the main reasons that I added fields to options is to add the fields that matlab has but absent from Octave.  Please consider this user case.   Thanks.

Liang Tang <lt1234>
Tue 08 Aug 2023 05:26:07 PM UTC, comment #9: 

Imho, the behavior after Rik's change is more consistent than what is proposed in comment #7.
`optimset` prevents such options from getting set now. So, `optimget` should behave the same way.
It is true that it's possible to add additional fields to the options structure like to any other structure. But in this case, it seems reasonable to me to expect from users to access these fields like they would do for any other structure (i.e., with `.`-indexing).
Anyway, it might be reasonable to discourage mixing direct structure access with using `optimset` and `optimget`. But that's a documentation issue and probably fits better in bug #64516.

Markus Mützel <mmuetzel>
Group administrator
Tue 08 Aug 2023 04:30:11 PM UTC, comment #8: 

I added a note to the NEWS file:
https://hg.savannah.gnu.org/hgweb/octave/rev/4379d4e29943

Feel free to adapt to potential follow-up changes.

Markus Mützel <mmuetzel>
Group administrator
Tue 08 Aug 2023 02:16:38 PM UTC, comment #7: 

I'm ok with set issuing an error, but would it make more sense for optimget to still issue a warning? it will return empty if it's ambiguous unless it's an exact match, at which point it will issue an warning but return the exact match value as well. Perhaps the warning message could be updated to mention the ambiguity and clearly re-state the name of the field being returned?

Nicholas Jankowski <nrjank>
Group Member
Tue 08 Aug 2023 02:03:21 PM UTC, comment #6: 

Sure, go ahead and add a note to the NEWS file.  The change was made on the development branch precisely because it might be de-stabilizing to existing code.  However, when programmers update major versions of Octave they can expect greater changes.

Rik <rik5>
Group administrator
Tue 08 Aug 2023 06:28:54 AM UTC, comment #5: 

This change might break compatibility with code written for older versions of Octave. (Code that previously succeeded with a warning now fails with an error.)
I agree that this change likely helps to catch unintentional user errors. Imho, it should be mentioned in the NEWS file though.

Markus Mützel <mmuetzel>
Group administrator
Mon 07 Aug 2023 06:25:31 PM UTC, comment #4: 

posts crossed. resetting tags.

Nicholas Jankowski <nrjank>
Group Member
Mon 07 Aug 2023 06:23:45 PM UTC, comment #3: 

looks like optimget returns an empty array unless something with the actual ambiguous name is present. since it's currently valid to set arbitrary fields, an error being issued might prevent querying a manually set parameter with optimget:


>> a = optimset('max_fract_change',1, 'max_rand_step',2, 'MaxFunEvals',3, 'MaxIter',4)
a =

  scalar structure containing the fields:

    max_fract_change = 1
    max_rand_step = 2
    MaxFunEvals = 3
    MaxIter = 4

>> optimget(a, 'max')
warning: optimget: ambiguous option: max (max_fract_change, max_rand_step, MaxFunEvals, MaxIter)
warning: called from
    optimget at line 55 column 5

ans = [](0x0)

>> a.max = 6
a =

  scalar structure containing the fields:

    max_fract_change = 1
    max_rand_step = 2
    MaxFunEvals = 3
    MaxIter = 4
    max = 6

>> optimget(a, 'max')
warning: optimget: ambiguous option: max (max_fract_change, max_rand_step, MaxFunEvals, MaxIter)
warning: called from
    optimget at line 55 column 5

ans = 6


Nicholas Jankowski <nrjank>
Group Member
Mon 07 Aug 2023 06:18:06 PM UTC, comment #2: 

Fixed in this changeset http://hg.savannah.gnu.org/hgweb/octave/rev/d1bbd7a0982c.

Closing report.

Rik <rik5>
Group administrator
Mon 07 Aug 2023 06:12:30 PM UTC, comment #1: 

I just discovered that optimget() has the same issue.  Changing the summary to reflect that.

Rik <rik5>
Group administrator
Mon 07 Aug 2023 06:02:57 PM UTC, original submission:  

Code to reproduce:


y = optimset ('max', 5)
warning: optimset: ambiguous option: max (MaxFunEvals, MaxIter)
warning: called from
    optimset>setoptionfields at line 191 column 9
    optimset at line 169 column 12

y =

  scalar structure containing the fields:

    max = 5


Octave warns about an ambiguous option name, but then proceeds to create a struct with the possibly incorrect name.  Matlab correctly emits an error and stops processing because it is simply unclear what the programmer intended for the example above.  I selected "Matlab Compatibility" for the item group, even though I also thing this is incorrect behavior.

I have a changeset for this which I will push shortly.

Rik <rik5>
Group administrator

 

(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 arungiridhar (Posted a comment)
  • -email is unavailable- added by lt1234 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by rik5 (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-09-21 arungiridhar StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2023-08-09 nrjank StatusIn Progress Ready For Test
    2023-08-08 mmuetzel StatusFixed In Progress
        Open/ClosedClosed Open
    2023-08-07 nrjank StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2023-08-07 nrjank StatusFixed Confirmed
        Open/ClosedClosed Open
    2023-08-07 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2023-08-07 rik5 Summaryoptimset issues warning, rather than error, when short name matches multiple possible options optimset/optimget issues warning, rather than error, when short name matches multiple possible options

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code