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

 
 

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

Submitted by:  Rik <rik5>
Submitted on:  Sun 16 Oct 2016 05:23:33 PM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Postponed Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Wed 16 Nov 2016 06:42:17 PM UTC, comment #13:

Marking as postponed until bug #45367 is solved.

Rik <rik5>
Project 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>
Project Administrator
Mon 24 Oct 2016 03:26:13 PM UTC, comment #11:

I benchmarked with the following code:

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

Results:

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>
Project 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

with patch

(file #38801)

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

Your patch has

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>
Project 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

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>
Project Member
Fri 21 Oct 2016 12:46:17 AM UTC, comment #7:

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

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>
Project 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).

Carnë Draug <carandraug>
Project 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.

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>
Project Administrator
Thu 20 Oct 2016 01:08:07 AM UTC, comment #4:

Rik,

I observed the following:

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

c.

(file #38770)

Carlo de Falco <cdf>
Project 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>
Project 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>
Project 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>
Project 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,

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

Test code:

Results:

Rik <rik5>
Project 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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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.3