bugGNU Octave - Bugs: bug #44193, whos "elements" count...

 
 

bug #44193: whos "elements" count wrong for user-defined class, capacity problem?

Submitter:  Colin Macdonald <cbm>
Submitted:  Thu 05 Feb 2015 10:56:13 PM UTC
   
 
Category:  Interpreter Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Inaccurate Result
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 19 Dec 2016 12:23:36 AM UTC, comment #10: 

I've confirmed that the scenario I described is what is happening.  That is, I'll write notes in the code below of what happens with the "x = A(rand(3,4)); whos" command:


      octave_value varval (context_id context = xdefault_context) const
      {
        if (is_global ())
          return symbol_table::global_varval (name);
        else if (is_persistent ())
          return symbol_table::persistent_varval (name);
        else
          {
            if (context == xdefault_context)
              context = active_context ();

            if (context < value_stack.size ())
              return value_stack[context];  // ALWAYS GOES THIS ROUTE
            else
              return octave_value ();  // NEVER GOES THIS ROUTE
          }
      }


I see the the line ALWAYS GOES THIS ROUTE accessed maybe 50 or 60 times with the command.  (I suppose there are many things in the symbol table beyond the user's variables.)

Beyond that, I've not looked further, mainly because of the slowness of doing development when touching any sort of header file.  Takes maybe 20 to 30 minutes to recompile in that case.  These routines longer than a few lines should not be inline and inside the header file.  Inline routines are for the case when the cost of jumping (i.e., setting up stack, making the jump, etc.) is large relative to the length of the routine.

Dan Sebald <sebald>
Sun 18 Dec 2016 10:52:21 PM UTC, comment #9: 

I lost track of this bug...

I no longer see a "elements += val.capacity ();" in variables.cc.  Perhaps a change was made there to fix bug #43096 but didn't get to checking this one.  Anyway, the line of code that computes number of elements and displays is now


        for (const auto& syminfo : lst)
          {
            syminfo.display_line (os, params);

            octave_value val = syminfo.varval;

            elements += val.numel ();
            bytes += val.byte_size ();
          }


It appears here that after extracting the A class files to a subdirectory and generating x, y, z according to comment #5, numel(x) produces a proper result.  The code that does this is (from data.cc):


  if (nargin == 1)
    retval = args(0).numel ();
  else if (nargin > 1)
    {
      // Don't use numel (const octave_value_list&) here as that corresponds to
      // an overloaded call, not to builtin!
      retval = dims_to_numel (args(0).dims (), args.slice (1, nargin-1));
    }


It seems to me that now both routes use the octave_value::numel() route.  So why does one produce a correct result and not the other?  The answer might be in the symbol_record::varval() routine, which is:


      octave_value varval (context_id context = xdefault_context) const
      {
        if (is_global ())
          return symbol_table::global_varval (name);
        else if (is_persistent ())
          return symbol_table::persistent_varval (name);
        else
          {
            if (context == xdefault_context)
              context = active_context ();

            if (context < value_stack.size ())
              return value_stack[context];
            else
              return octave_value ();
          }
      }


That is, whereas the numel() function definition uses the octave_value "arg(0)" directly, this symbold_record::varval() routine might be returning something other than the symbol's octave_value depending on active_context().  Just a theory.  I'll experiment after compiling finishes.

Dan Sebald <sebald>
Sun 18 Dec 2016 08:38:51 PM UTC, comment #8: 

I think this behavior is still present in Octave 4.2.0.

The result from the code in comment #5 is still the same. (Even though the mentioned bug #43096 is now marked as fixed.)

Hartmut <hardy>
Fri 06 Feb 2015 09:37:43 PM UTC, comment #7: 

I would expect this is related to the kluge mentioned in https://savannah.gnu.org/bugs/?43096, so a proper fix there would likely resolve this.

Colin Macdonald <cbm>
Fri 06 Feb 2015 08:28:25 PM UTC, comment #6: 

OK, I understand it now.

Yeah, I tried searching too and I see the "elements += val.capacity ();" line in variables.cc.  And searching through octave_value indicates that capacity() is basically numel() in the base class and sometimes is nelem().

But I'm having trouble grepping for "class" definition because, well, "class" is used a thousand times in the code.  I think "class" is something new.  Is it this routine:


octave_idx_type
octave_class::numel (const octave_value_list& idx)
{


in libinterp/octave-value/ov-class.cc?

Dan Sebald <sebald>
Fri 06 Feb 2015 10:34:54 AM UTC, comment #5: 

Let's start again so its all in this bug instead of cross-linked to the other one where @mtmiller pointed this out.  See attached class.


octave:1> x = A(rand(3,4))
class is: A
 with data:
   0.191086   0.658946   0.063937   0.394757
   0.285017   0.690299   0.827248   0.889642
   0.378245   0.180257   0.350445   0.330926
octave:2> y = A(42)
class is: A
 with data:
 42
octave:3> z = 42
z =  42
octave:4> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        x           3x4                         96  A
        y           1x1                          8  A
        z           1x1                          8  double

Total is 3 elements using 112 bytes


Note each x and y counts as only one "element" even though x reports numel(x) -> 12 and size(x) -> [3 4].

I.e., the element calculation does not respect numel/size/etc for a user-defined class.  From reading the code, the element count is done by adding up the ".capacity".  That is as far as I've tracked it down.

Colin Macdonald <cbm>
Fri 06 Feb 2015 10:27:07 AM UTC, comment #4: 

I take it back, I think the current sparse behaviour looks correct to me.  The count matches nnz() of the sparse matrix.  That's what I expect.

Colin Macdonald <cbm>
Fri 06 Feb 2015 10:24:33 AM UTC, comment #3: 

yeah, sorry "symbolic".  Any simple class will demonstrate it, see @A class in the linked bug report.

As you note, apparently sparse shows it too.

Colin Macdonald <cbm>
Fri 06 Feb 2015 09:33:17 AM UTC, comment #2: 

What is 'sym'?  Is it related to symmetric elimination tree?  "symbolic"?  Here's another example:


>> sparse(1,2,3)
ans =

Compressed Column Sparse (rows = 1, cols = 2, nnz = 1 [50%])

  (1, 2) ->  3

>> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        ans         1x2                         24  double

Total is 1 element using 24 bytes


Could it be that the sparse variables only count the defined, non-zero values?  Filling in the zero value results in two elements:


>> ans(1,1) = 6
ans =

Compressed Column Sparse (rows = 1, cols = 2, nnz = 2 [100%])

  (1, 1) ->  6
  (1, 2) ->  3

>> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        ans         1x2                         36  double

Total is 2 elements using 36 bytes


Dan Sebald <sebald>
Thu 05 Feb 2015 10:57:05 PM UTC, comment #1: 

This is a minor bug (I don't seem to be able to set that?)

Colin Macdonald <cbm>
Thu 05 Feb 2015 10:56:13 PM UTC, original submission:  

In this example, the count should be 9 (from the doubles) + 7 (from the user-defined class sym).  But instead each sym counts only as 1 and we see 11 displayed.


>> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        A           3x2                        240  sym
        d           3x3                         72  double
        x           1x1                         30  sym

Total is 11 elements using 342 bytes


@mtmiller found this looking at a related bug:

https://savannah.gnu.org/bugs/?43096

There is a minimal class that demonstrates this problem in that bug.

In that other bug, I said:

> The number of elements thing is counted by adding up
> the ".capacity" of each varval. So I guess "capacity"
> is wrong for simple classes.


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 #33015:  A.tar added by cbm (10KiB - application/x-tar - a simple class that demostrates this bug)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-06 mtmiller Severity3 - Normal 2 - Minor
        Item GroupNone Inaccurate Result
        StatusNone Confirmed
    2015-02-06 cbm Attached File- Added A.tar, #33015

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code