[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Nicholas Jankowski
Subject: [Octave-patch-tracker] [patch #10288] Implementation of Matlab function tensorprod
Date: Mon, 24 Oct 2022 11:47:34 -0400 (EDT)

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

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:


Message sent via Savannah

reply via email to

[Prev in Thread] Current Thread [Next in Thread]