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

 
 

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

Submitted by:  Colin Macdonald <cbm>
Submitted on:  Thu 28 Aug 2014 08:49:14 AM UTC  
 
Category: GUISeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating 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 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>
Project 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>
Project 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.

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

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.

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.

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

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

and I guess you should implement dims with something like

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 File(s):
   
   
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
  • -unavailable- added by rik5
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by jordigh (Posted a comment)
  • -unavailable- added by cbm (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 15 Feb 2015 03:49:57 PM UTCrik5StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Fri 06 Feb 2015 03:47:42 PM UTCrik5Carbon-Copy-=>Added jwe
    Thu 05 Feb 2015 10:48:54 PM UTCcbmAttached File-=>Added gui_size_workaround.diff, #33011
    Thu 05 Feb 2015 09:23:19 PM UTCcbmAttached File-=>Added A.tar, #33009
    Thu 05 Feb 2015 09:21:24 PM UTCcbmAttached File-=>Added size.m, #33006
      Attached File-=>Added display.m, #33007
      Attached File-=>Added A.m, #33008
    Mon 02 Feb 2015 04:39:46 PM UTCjordighStatusNeed Info=>Confirmed
    Mon 02 Feb 2015 04:38:46 PM UTCjordighStatusNone=>Need Info
    Thu 28 Aug 2014 08:49:14 AM UTCcbmAttached File-=>Added Screenshot from 2014-08-28 09:46:11.png, #31988

    Back to the top


    Powered by Savane 3.1-cleanup1