[Octave-bug-tracker] [bug #51308] ndims, rows, columns incorrect for classes that only overload size

[Kai Torben Ohlhus]

Follow-up Comment #3, bug #51308 (project octave):

Mike, I see your concerns.  My arguments to speed up the process (despite
personal interest ;-) ) are best explained by tracing down the change for the
"isvector" function [1], that effectively nothing changes:

Current:

dim_vector sz = args(0).dims ();
return ovl (sz.ndims () == 2 && (sz(0) == 1 || sz(1) == 1));


Proposed:

Matrix sz = octave_value (args(0)).size ();
return ovl (sz.numel () == 2 && (sz(0) == 1 || sz(1) == 1));


Thus "dims" is only replaced by "size", which, in the absence of undiscovered
bugs, returns the exact same value (in a different data container Matrix vs.
dim_vector).  Further "dims" is constant, while "size" is not (due to
undocumented but comprehensible facts [2]) and makes a octave_value-wrapping
necessary.  On the other hand, "size" is overloadable.

- The aforementioned functions might be marginally slower, due to the wrapping
(no figures to prove or disprove this).  But compiler optimization should cope
with the situation for builtin types.

+ The change affects only the pure interpreter functions "isscalar",
"isvector", ... without liboctave changes, just by calling the "same"
function.

+ For builtin datatypes nothing can change (the code is compiled), we only
improve the situation for user-defined classes.

+ The change comes in a few months, rather than next year (some users, among
them Forge developers, are waiting since 2015 for this change).

If you or others still disagree, I will just push it to default and will do
nothing before getting feedback.


[1]
https://hg.savannah.gnu.org/hgweb/octave/file/868830474750/libinterp/corefcn/data.cc#l3651
[2] It took me a while to understand this.  For builtin datatypes, this is
pure nonsense.  Why should polling the size of an object change it's contents?
 In 2006, when jwe introduced this function [3], it was constant.  Later in
2009 Jaroslav Hajek removed const, to enable user-defined classes [4].  He
needed to call the interpreter "feval" [5] for evaluating the user defined
"size" method.  And when calling the interpreter, the control over the object
is given away, thus it might change (the user can delete data in her size
function for example) and a copy has to be made, which indeed changes the
state of the object.  I don't know if there was a better design possible, but
maybe I will add this comment as future reference, why the non-const-ness is
necessary at the moment.
[3] https://hg.savannah.gnu.org/hgweb/octave/rev/960f4b9a26af#l8.7
[4] https://hg.savannah.gnu.org/hgweb/octave/rev/67fc970dad7d#l9.8
[5] https://hg.savannah.gnu.org/hgweb/octave/rev/67fc970dad7d#l6.16

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?51308>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/