bugGNU Octave - Bugs: bug #43097, if (object) should call if...

 
 

bug #43097: if (object) should call if (logical(object))

Submitter:  Colin Macdonald <cbm>
Submitted:  Thu 28 Aug 2014 09:00:16 AM UTC
   
 
Category:  Interpreter 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:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 18 May 2016 06:12:23 PM UTC, comment #8: 

Since the patch works for you, and for some minimal testing here, and I don't see any performance degradations here or any test failures, pushed to default,

http://hg.savannah.gnu.org/hgweb/octave/rev/78e0b187904b

Mike Miller <mtmiller>
Group Member
Tue 17 May 2016 06:40:11 PM UTC, comment #7: 

yeah, hg tip makes symbolic slow with or without this patch...  That must be some other issue: a chance for me to learn how to bisect in hg perhaps: but not today!

Colin Macdonald <cbm>
Tue 17 May 2016 06:14:29 PM UTC, comment #6: 

Thanks for testing. FWIW I get about the same time in both 4.0.1 and the default branch (with the patch applied) running on octsympy git master:


$ octave-4.0.1 foo.m

PASSES 11 out of 11 tests
Elapsed time is 3.74033 seconds.
PASSES 41 out of 41 tests
Elapsed time is 9.3135 seconds.

$ octave-dev foo.m

PASSES 11 out of 11 tests
Elapsed time is 3.2307 seconds.
PASSES 41 out of 41 tests
Elapsed time is 9.22395 seconds.


Mike Miller <mtmiller>
Group Member
Tue 17 May 2016 06:02:48 PM UTC, comment #5: 

Actually, something is making my tests about 4 times slower...

4.0.1:

>> tic; test @sym/subs; toc
PASSES 11 out of 11 tests
Elapsed time is 6.66044 seconds.
>> tic; test @sym/subsasgn; toc
PASSES 41 out of 41 tests
Elapsed time is 17.5581 seconds.


hg tip with this patch:

>> tic; test @sym/subs; toc
PASSES 11 out of 11 tests
Elapsed time is 22.781 seconds.
>> tic; test @sym/subsasgn; toc
PASSES 41 out of 41 tests
Elapsed time is 64.7933 seconds.


I'll need to test hg tip without this patch...

Colin Macdonald <cbm>
Tue 17 May 2016 05:49:52 PM UTC, comment #4: 

I can't vouch for correctness, but this works great with the symbolic pkg!

Colin Macdonald <cbm>
Tue 17 May 2016 04:33:35 PM UTC, comment #3: 

So this is for old-style classes defined by an @class directory I suppose. Objects implemented in C++ can override the octave_base_value::is_true method for this purpose.

As far as I can tell, this would be implemented by adding a method octave_class::is_true and having it look up and evaluate the "logical" method. Something like the attached diff seems to work for me.

(file #37187)

Mike Miller <mtmiller>
Group Member
Thu 29 Jan 2015 12:29:29 AM UTC, comment #2: 

A similar example: subasgn into a double array:


x = sym(2)
y = [0 0]
y(1) = x
error: operator =: no conversion for assignment of 'class' to indexed 'matrix'
error: assignment failed, or no method for 'matrix = class'


This works fine in Matlab, which does y(1) = double(x) automatically.

Colin Macdonald <cbm>
Thu 28 Aug 2014 09:02:18 AM UTC, comment #1: 

Hmm, perhaps that example is not convincing given weird other "True" value.  Here's a better one, which should convince you we should fix this :)


>> pkg load octsympy
>> q = sym(10)
q = (sym) 10
>> if (q), disp("hello"); end
>> if (logical(q)), disp("hello"); end
hello


Nonzero should be true, great potential to confuse users!

Colin Macdonald <cbm>
Thu 28 Aug 2014 09:00:16 AM UTC, original submission:  

Suppose I have a class that can convert itself to boolean (implements the "logical" method).

If I write "if (object)", Matlab calls "if (logical(object))" automatically.  We do not.

Example using octsympy:


>> pkg load octsympy
>> syms x
>> e = (x == x)
e = (sym) True
>> if (e), disp("hello"); end
>> if (logical(e)), disp("hello"); end
hello
>>


On matlab, both would produce "hello".

(octsympy currently does not autoconvert its own internal format to logical type, that might change in the future but that does not change this bug).

Colin Macdonald <cbm>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #37187:  bug43097.diff added by mtmiller (1KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller (Updated the item)
  • -email is unavailable- added by cbm (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-05-18 mtmiller StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-05-17 mtmiller Attached File- Added bug43097.diff, #37187
        StatusNone Patch Submitted

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code