[Octave-patch-tracker] [patch #10288] Implementation of Matlab function tensorprod

[Nicholas Jankowski]

Follow-up Comment #8, patch #10288 (project octave):

running commentary:

try to limit line lengths to 80 characters. long strings can be
split/concatenated across multiple lines using [,], long lines can be split
with ...

i think if !isequal(size(A),size(B)) could be replaced with if
any(size(A)!=size(B)) to avoid the more expensive call to isequal.

your dim checks aren't actually checking if the inputs are integer valued. i
think a==fix(a) is a fairly low overhead check for this but others might know
a better method.

try to group similar tests.  e.g., the 'Invalid call to' tests are usually
grouped at the top, followed by individual error message testing. And it's not
a hard rule, but a specific error message is usually more informative to the
reader than print_usage, as it doesn't point out the actual error. We often
limit it to the 'incorrect number of inputs' type errors. e.g., in the ischar
check followed by the 'is it 'all'' check, it would be better to report back
something like error("unknown parameter %s", varargin{1}) to actually point
them to the specific error.  if it fails the isnumeric and ischar tests, an "A
must be of type...." error would be more informative than print_usage.

in the initial error messages, I'd probably use the phrase "A must be a
numeric array" vs tensor, as 'array' is the name of that type of numerical
element.

i can try a series of empty and nan input shape tests later to verify matlab
outputs, in addition to confirming your other tests.

 



  



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?10288>

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