bugGNU Octave - Bugs: bug #43096, GUI workspace pane shows dimension...

 
 

bug #43096: GUI workspace pane shows dimension 1x1 for my class

Submitter:  Colin Macdonald <cbm>
Submitted:  Thu 28 Aug 2014 08:49:14 AM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
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

Sun 15 Feb 2015 04:00:13 PM UTC, comment #21: 

Ok, but this patch was kind messy though...  I could make you one like bug #44201 (but without the controversial nan/inf stuff).

Personally, I'd rather stick with 1x1 than see -247123123x3.  So my preference for 4.0 is #44201 or neither.  But its not a strong preference.

Colin Macdonald <cbm>
Sun 15 Feb 2015 03:49:57 PM UTC, comment #20: 

I applied the temporary patch from this bug report (http://hg.savannah.gnu.org/hgweb/octave/rev/675d92e7d507).  I think the changes in bug #44201 are more far-reaching and require greater time to consider than we have before the 4.0 release.

Rik <rik5>
Group administrator
Tue 10 Feb 2015 10:48:33 AM UTC, comment #19: 

@rik5: in https://savannah.gnu.org/bugs/?44201, I have a patch that calls get_dims_str() in symtab.cc.  That is probably nicer than my patch here.  Then we can still try to fix the kluge independently of this particular GUI bug.

Colin Macdonald <cbm>
Sat 07 Feb 2015 05:38:59 PM UTC, comment #18: 

I want to wait a bit and see if we can fix this for real.  If we can't then I think it is acceptable to put the kluge in place for 4.0.

Rik <rik5>
Group administrator
Sat 07 Feb 2015 02:58:30 PM UTC, comment #17: 

See also https://savannah.gnu.org/bugs/?44201 where I further patch it to work with inf/nan sizes.

Colin Macdonald <cbm>
Fri 06 Feb 2015 09:39:16 PM UTC, comment #16: 

Agreed, but unless that can happen for 4.0, can we put in my patch?  (i.e., the whos kludge copy-pasted to the GUI code)

Colin Macdonald <cbm>
Fri 06 Feb 2015 03:52:27 PM UTC, comment #15: 

I added jwe to the CC list for this bug since he put in the original kluge in 2009.


changeset:   9704:bb413c0d0d6d
user:        John W. Eaton <jwe@octave.org>
date:        Wed Oct 07 14:34:53 2009 -0400
summary:     whos: kluge fix to get size right for objects


It would be really nice to fix this correctly as suggested by the comment in the code that was re-quoted in comment #11.

If that were done, then isrow, iscolumn, isvector, isscalar would automatically just work for instances of class objects.  The change that you mention happened because those functions have been promoted to C++ for speed and efficiency.  The old m-file implementations used to call size() which was effectively overloaded for class objects.

Rik <rik5>
Group administrator
Thu 05 Feb 2015 10:48:54 PM UTC, comment #14: 

Patch attached.

(file #33011)

Colin Macdonald <cbm>
Thu 05 Feb 2015 10:18:37 PM UTC, comment #13: 

The kludge works :)  I will prepare a patch.  But of course a real fix is good too.  Or if anyone knows a bug report for that kludge, we can reference it in the code and here as this is sort of a dupe.

Colin Macdonald <cbm>
Thu 05 Feb 2015 09:43:46 PM UTC, comment #12: 

So we can either do a similar kludge in symtab.cc, or fix .dims() ("unresolved const issues"?) to do the right thing.

Former is potentially do-able by me, I probably don't have time/experience for latter...

Colin Macdonald <cbm>
Thu 05 Feb 2015 09:37:07 PM UTC, comment #11: 

Re: @mtmiller's "total elements" thing.  I looked at the whos implementation in libinterp/corefcn/variables.cc.  The number of elements thing is counted by adding up the ".capacity" of each varval.  So I guess "capacity" is wrong for simple classes.

Also, whos uses "get_dims_str" to render the "3x4" size bit.  This code says:


// FIXME: This is a bit of a kluge.  We'd like to just use val.dims()
// and if val is an object, expect that dims will call size if it is
// overloaded by a user-defined method.  But there are currently some
// unresolved const issues that prevent that solution from working.


Hopefully that is helpful to someone tracking this down...

Colin Macdonald <cbm>
Thu 05 Feb 2015 09:25:12 PM UTC, comment #10: 

I can also confirm @mtmiller's whos "Total is 1 element" issue.  This class demonstrates that too.  Let me know if you want a separate issue for that.

Colin Macdonald <cbm>
Thu 05 Feb 2015 09:21:24 PM UTC, comment #9: 

Ok, I'll post.  Put the three .m files in a dir called "@A".

This "A" class only has A.m, display.m and size.m. 

But adding methods for numel, rows and columns does not solve this.
I'll upload the more complete example with these as a tarball.


(file #33006, file #33007, file #33008)

Colin Macdonald <cbm>
Thu 05 Feb 2015 03:38:37 AM UTC, comment #8: 

Confirmed with the interval package, but not with other classes, e.g. ftp or strel from the image package.


>> pkg load interval
>> x = infsup ()(ones (4));
>> whos x
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        x           4x4                        256  infsup

Total is 1 element using 256 bytes


but the workspace pane shows the dimensions as "1x1".

The equivalent for the @ftp or @strel classes shows the correct dimensions in the workspace pane, e.g.


>> f = ftp();
>> fff = [f f f];
>> whos fff
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        fff         1x3                         72  ftp

Total is 3 elements using 72 bytes


Notice that the last line differs, the ones that show the correct dimension in the workspace pane say "Total is N elements", while the interval example that doesn't says "Total is 1 element".

Mike Miller <mtmiller>
Group Member
Thu 05 Feb 2015 02:49:13 AM UTC, comment #7: 

@Colin: Can you post a simple test case that we can use for debugging?

Rik <rik5>
Group administrator
Mon 02 Feb 2015 04:39:46 PM UTC, comment #6: 

Oh, sorry, you did this on 3.8.1, so it's for an old-style class.

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Mon 02 Feb 2015 04:38:46 PM UTC, comment #5: 

Is this for a classdef class or for an old-style class?

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Sun 01 Feb 2015 10:39:26 PM UTC, comment #4: 

I noticed this also affects the "interval" package, so its not just me :)

Colin Macdonald <cbm>
Thu 23 Oct 2014 08:54:23 AM UTC, comment #3: 

Is there some basic array object I was supposed to subclass?  I seems that Octave thinks my class is scalar and I'm hacking around at the m-file level to make it behave like an array.

Nonetheless, the fact the "whos" shows the dimenions from size.m makes me feel on some level this is an Octave bug.

Colin Macdonald <cbm>
Thu 23 Oct 2014 08:50:04 AM UTC, comment #2: 

I have a pure m-file class.  It has no explicit subclass (so I would guess that is "octave_base_value" but I don't know yet).

So it looks like the information I provide in numel.m/size.m is not getting to the C .dims stuff.

Some other evidence this is occuring is that I had to override isrow, iscolumm, isvector and isscalar even though I have a proper numel.m and size.m

I'll try to dig a bit deeper when I have time.  But if anyone wants to follow up in the meantime, the downstream bug is:

https://github.com/cbm755/octsympy/issues/89

(but its probably easy to construct a smaller class that exhibits this).

Colin Macdonald <cbm>
Wed 22 Oct 2014 05:42:46 PM UTC, comment #1: 

I think it's in libinterp/corefcn/symtab.cc:symbol_table::do_workspace_info:


  dim_vector dv = val.dims ();


(http://hg.octave.org/octave/file/ce9bd5ed44d2/libinterp/corefcn/symtab.cc#l1407)

and I guess you should implement dims with something like

 dim_vector dims (void) const { dim_vector dv (youdfirstdim, yourseconddim); return dv; }


for you class (which I guess is derived from octave_base_value, right?)

Anonymous
Thu 28 Aug 2014 08:49:14 AM UTC, original submission:  

How does it get this information?  I've overloaded size, isscalar, numel,...

"whos" shows the correct dimensions.

I noticed between 3.8.1 and hg June-ish that I had to overload isscalar (in 3.8.1, it calls size or numel or something).  Perhaps that is related.

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 #33011:  gui_size_workaround.diff added by cbm (2KiB - text/x-patch - patch to workaround)
file #33009:  A.tar added by cbm (10KiB - application/x-tar - more complete example with numel, rows, columns)
file #33006:  size.m added by cbm (247B - text/x-objcsrc - example class)
file #33007:  display.m added by cbm (99B - text/x-objcsrc - example class)
file #33008:  A.m added by cbm (71B - text/x-objcsrc - example class)
file #31988:  Screenshot from 2014-08-28 09:46:11.png added by cbm (110KiB - image/png - Note whos versus workspace pane)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by jordigh (Posted a comment)
  • -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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-15 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2015-02-06 rik5 Carbon-Copy- Added jwe
    2015-02-05 cbm Attached File- Added gui_size_workaround.diff, #33011
    2015-02-05 cbm Attached File- Added A.tar, #33009
    2015-02-05 cbm Attached File- Added size.m, #33006
        Attached File- Added display.m, #33007
        Attached File- Added A.m, #33008
    2015-02-02 jordigh StatusNeed Info Confirmed
    2015-02-02 jordigh StatusNone Need Info
    2014-08-28 cbm Attached File- Added Screenshot from 2014-08-28 09:46:11.png, #31988

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code