bugGNU Octave - Bugs: bug #49364, odeset does not implement...

 
 

bug #49364: odeset does not implement non-exact match to property name.

Submitter:  Rik <rik5>
Submitted:  Sun 16 Oct 2016 05:23:33 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
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:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 20 Apr 2023 01:41:21 AM UTC, comment #14: 

I implemented PartialMatching in the inputParser class (bug #64050).  odeset() now inherits the ability to do non-exact matching of property names in the same way as Matlab.  I changed the BIST tests to pass since inputParser generates slightly different error messages now.  See http://hg.savannah.gnu.org/hgweb/octave/rev/b4fbbd9cf601.

Marking bug as Fixed and closing report.

Rik <rik5>
Group administrator
Wed 16 Nov 2016 06:42:17 PM UTC, comment #13: 

Marking as postponed until bug #45367 is solved.

Rik <rik5>
Group administrator
Tue 25 Oct 2016 12:21:51 AM UTC, comment #12: 

I added a dependency on bug #45367 with is a report on adding PartialMatching to inputParser.

Rik <rik5>
Group administrator
Mon 24 Oct 2016 03:26:13 PM UTC, comment #11: 

I benchmarked with the following code:


N = 400;

bm = zeros (N,1);

for i = 1:N;
  tic;
  x = odeset ("RelTol", 1e-1);
  #x = odeset ("FooBar", 1e-1);
  bm(i) = toc;
endfor


And then median(bm) to not have a few outliers change the result.

Results:


Old code: 0.0026391
New code: 0.0026280


Basically the same.  I don't trust that my computer is accurately timing events down to microsecond precision anyways.  I left the code as is.

Rik <rik5>
Group administrator
Mon 24 Oct 2016 10:54:00 AM UTC, comment #10: 


oops, you are right, sorry!
Does the new attached patch look correct to you?

With this I see a suspiciously substantial speed-up ...

without patch

>> tic, test odeset, toc
PASSES 9 out of 9 tests
Elapsed time is 0.443603 seconds.


with patch

>> tic, test odeset, toc
PASSES 9 out of 9 tests
Elapsed time is 0.124976 seconds.




(file #38801)

Carlo de Falco <cdf>
Group Member
Fri 21 Oct 2016 03:26:21 PM UTC, comment #9: 

Your patch has


+    if (! isempty (p.Unmatched))
+      ## Merge extra fields into existing odestruct
+      odestruct_extra = p.Unmatched;
+      xtra_fields = fieldnames (odestruct_extra);


but p.Unmatched is a scalar struct of size 1x1 (by definition, not empty) so this branch of the code will always be taken.  That's why I changed it to depend on fieldnames to make sure the struct was actually empty.

Rik <rik5>
Group administrator
Fri 21 Oct 2016 01:43:08 PM UTC, comment #8: 

Rik,

I was aware of the behaviour of isempty on structs
but in my patch I was testing


isempty (p.Unmatched)


as p.Unmatched is set to an empty struct before p.parse ()
it should be empty if and only if no unmatched parameters are found.





Carlo de Falco <cdf>
Group Member
Fri 21 Oct 2016 12:46:17 AM UTC, comment #7: 

True, and so to be completely general one would have to code


numel (x) == 0 || isempty (fieldnames (x))


However, for this case there is no need since the structure is directly returned from inputParser and hasn't had the deletions necessary to trigger the issue in question.


Rik <rik5>
Group administrator
Thu 20 Oct 2016 07:43:03 PM UTC, comment #6: 


> However, if you use fieldnames then both "empty" structures above report that they are empty.


This alone will not work if the struct had fields before and was then "emptied" (depending on how define empty struct).


octave:1> x.foo = 1;
octave:2> x(1) = []
x =

  1x0 struct array containing the fields:

    foo

octave:3> fieldnames (x)
ans =
{
  [1,1] = foo
}


Carnë Draug <carandraug>
Group Member
Thu 20 Oct 2016 03:18:37 PM UTC, comment #5: 

@Carlo: I tried "isempty (odestruct_extra)" the first time I recoded the routine, but discovered a quirk.  isempty() returns true if the number of elements is 0.  Thus a scalar structure is a 1x1 object and has one element, even if it has no field names.  This is not what most people mean when they say "the structure is empty".  See the following code in Octave which is unintuitive.


octave:1> x = struct ()
x =

  scalar structure containing the fields:


octave:2> size (x)
ans =

   1   1

octave:3> isempty (x)
ans = 0
octave:4> y = struct ([])
y =

  0x0 struct array containing the fields:


octave:5> size (y)
ans =

   0   0

octave:6> isempty (y)
ans = 1


However, if you use fieldnames then both "empty" structures above report that they are empty.  Searching around on the Internet I found that the fieldnames method is, indeed, the recommended way to test structures for being empty.

Rik <rik5>
Group administrator
Thu 20 Oct 2016 01:08:07 AM UTC, comment #4: 


Rik,

I observed the following:


>> tic, isempty (odestruct_extra); iet = toc
iet =    3.3140e-05
>> tic, xtra_fields = fieldnames (odestruct_extra); ! (isempty (xtra_fields)); ieft =  toc
ieft =    1.9383e-04
>> tic, a = odeset (); ost =  toc
ost =  0.0055351


So the attached change may improve performance by a few more hundreds of microseconds.

c.


(file #38770)

Carlo de Falco <cdf>
Group Member
Wed 19 Oct 2016 07:13:29 PM UTC, comment #3: 

I made a change so that Octave emits a warning when an unknown property is used (http://hg.savannah.gnu.org/hgweb/octave/rev/c28648e039da).

Changing report status back to confirmed and release to dev.  The final fix should take place on the development branch.

Rik <rik5>
Group administrator
Sun 16 Oct 2016 07:49:20 PM UTC, comment #2: 

From e-mail, workaround #1 depends on support for non-exact matching in inputParser class.  Temporary fix should be workaround #2, to issue a warning.

I added notes to odeset.m to document this (http://hg.savannah.gnu.org/hgweb/octave/rev/0b21aece4b93).

Rik <rik5>
Group administrator
Sun 16 Oct 2016 05:47:45 PM UTC, comment #1: 

Possible workarounds include:

1) Implementing non-exact match in Octave
2) Generating a warning when a property name does not match a known value

Rik <rik5>
Group administrator
Sun 16 Oct 2016 05:23:33 PM UTC, original submission:  

In Matlab, odeset will match a partial property name to the exact property name.  Octave does not do this.  Instead, it creates a new property under the abbreviated name.

This is a problem.  Code on Matlab which makes use of this feature does not generate any warning, therefore, there is no indication that the option should be written more specifically.  When this code is ported to Octave, it will run, but differentent results will be obtained because some options have not been set identically.  As an example,


odeset ('Rel', 1e-12)


will set RelTol to a very strict 1e-12 in Matlab, but leave it at the default value in Octave.

Test code:


% Should ignore case and set RelTol, is list of properties still alphabetic?
x = odeset ('reltol', 1e-1)
% Does Matlab error or warn when option value is incorrect?
y = odeset ('RelTal', 1e-2)
% Does Matlab auto-complete to nearest available property as for graphics?
z = odeset ('Rel', 1e-3)
% Can arbitrary properties be added?
w = odeset ('MyCustomProperty', 1)


Results:


These are the outputs on r2016a:

x = odeset ('reltol', 1e-1)

x =

               AbsTol: []
                  BDF: []
               Events: []
          InitialStep: []
             Jacobian: []
            JConstant: []
             JPattern: []
                 Mass: []
         MassSingular: []
             MaxOrder: []
              MaxStep: []
          NonNegative: []
          NormControl: []
            OutputFcn: []
            OutputSel: []
               Refine: []
               RelTol: 0.1000
                Stats: []
           Vectorized: []
     MStateDependence: []
            MvPattern: []
         InitialSlope: []

___________________

y = odeset ('RelTal', 1e-2)
Error using odeset (line 225)
Unrecognized property name 'RelTal'.

__________________

z = odeset ('Rel', 1e-3)

z =

               AbsTol: []
                  BDF: []
               Events: []
          InitialStep: []
             Jacobian: []
            JConstant: []
             JPattern: []
                 Mass: []
         MassSingular: []
             MaxOrder: []
              MaxStep: []
          NonNegative: []
          NormControl: []
            OutputFcn: []
            OutputSel: []
               Refine: []
               RelTol: 1.0000e-03
                Stats: []
           Vectorized: []
     MStateDependence: []
            MvPattern: []
         InitialSlope: []

_______________________

w = odeset ('MyCustomProperty', 1)
Error using odeset (line 225)
Unrecognized property name 'MyCustomProperty'.



Rik <rik5>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #38801:  open_6piZ7uDQ.txt added by cdf (983B - text/plain)
file #38770:  open_uCvbvvqp.txt added by cdf (622B - text/plain)
file #38745:  odeset_merge.cset added by rik5 (2KiB - application/octet-stream)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by cdf (Updated the item)
  • -email is unavailable- added by rik5
  • -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-04-20 rik5 Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0
    2023-04-20 rik5 StatusPostponed Fixed
    2016-11-16 rik5 StatusConfirmed Postponed
    2016-10-25 rik5 Dependencies- Depends on bugs #45367
    2016-10-24 cdf Attached File- Added open_6piZ7uDQ.txt, #38801
    2016-10-20 cdf Attached File- Added open_uCvbvvqp.txt, #38770
    2016-10-19 rik5 StatusPatch Submitted Confirmed
        Release4.2.0-rc2 dev
    2016-10-18 rik5 StatusConfirmed Patch Submitted
    2016-10-17 rik5 Attached File- Added odeset_merge.cset, #38745
    2016-10-17 rik5 Carbon-Copy- Added cdf

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code