[Octave-bug-tracker] [bug #50007] NaN aware min and max via "includenan"

[Stefan Mirea]

Follow-up Comment #1, bug #50007 (project octave):

I implemented the "includenan" flag for min/max and cummin/cummax in two ways:
(mainly) in liboctave and (mainly) in libinterp (see the list
<http://octave.1599824.n4.nabble.com/The-nanflag-parameter-td4681462.html>).
Read on for more verbosity.

Concerning the liboctave solution: in the "includenan" case in OP_MINMAX_FCN2,
while going through the rows first could make it possible to break the column
loop at the first NaN found, I chose to go through the columns first for
cache-friendliness and used an OCTAVE_LOCAL_BUFFER to know when to skip rows.
A quite similar method was used in OP_ROW_SHORT_CIRCUIT, but the number of
buffer updates could reach m * n there (if PRED is always false), while in my
case it is at most m. On the other hand, I am not cache-friendly on the buffer
itself.

As opposed to my response on the list regarding the libinterp solution:

* It was not necessary to manually go through the array to find the indices of
the original NaN values. Instead, I obtained them by calling min/max on the
result of isnan, converted to an int8NDArray. Note that NaN values can
actually differ for both real (NaN vs. NA) and complex (NaN+2i, NaN+3i, NA+3i)
types.

* For two inputs, I did not create all those arrays (x_extended,
zeros(size(y)), y_extended, zeros(size(x)) or x_extended_isnan), but called
do_bsxfun_op with my own functions which preserve NaNs.

* For two complex input arrays, I did not implement an additional "omitnan"
case in libinterp, but modified the underlying octave::math::min/max to omit
NaNs for complex types (as with real types) and implemented an "includenan"
case in libinterp. I checked that no code using octave::math::min/max relied
on their "includenan" comportment for complex types. Apart from the min/max
functions, they are only called with complex arguments from
__accumarray_min/max__, but even there including NaNs is wrong, not just
because of ML incompatibility, but it also produces bugs:

>> % calling with FUNC = @max and VALS including a NaN makes all the values in
the result NaN
>> accumarray ([1; 1; 2; 2], [1; 2i; 3; NaN], [], @max)
ans =

   NaN
   NaN

>> % calling with FUNC = @min and just complex VALS makes all the values in
the result NaN
>> accumarray ([1; 1; 2; 2], [i; 2i; 3; 4], [], @min)
ans =

   NaN
   NaN


Both patches will also fix the following issues:

* The min/max default behaviour is not "omitnan" when called with a
Sparse[Complex]Matrix on the second dimension and one of the rows has all the
elements nonzero and the first one NaN:

min (sparse ([1 1], [1 2], [NaN 1]), [], 2) % expected sparse (1), got sparse
(NaN)


* When calling min/max for a SparseComplexMatrix on the first dimension and
one of the columns has all the elements NaN, the result is 0 for that column:

min (sparse ([1 2], [1 1], [NaN+i NaN])) % expected sparse (NaN+i), got sparse
(0)


* OP_MINMAX_FCN2 and OP_CUMMINMAX_FCN2 in mx-inlines.cc were supposed to jump
to the faster code (according to the comment above them) as soon as r
contained no NaN value, but the actual code waits to find in v a column
without NaNs. For the following matrix:

max ([1 NaN 3; NaN 2 NaN], [], 2)

the third column can already be processed in the faster loop.

* Complex numbers with equal magnitudes are not ordered by phase angles when
calling min/max with a single SparseComplexMatrix or with two arguments,
either full or sparse.

* The min/max functions between a *Matrix and a scalar were implemented to
call the scalar-matrix versions, although the operations are not commutative:
between two NaN values, the first one should be returned (regardless of the
NaN flag).

* In the min/max functions between two [Float]ComplexMatrices, it is checked
for each column if all of the elements are real, in which case only the real
parts are compared. Therefore, negative numbers on real columns will not be
compared by magnitude.

* mx_inline_xmin/xmax between a real NaN scalar and a vector keep the NaN
values in the vector even though the scalar is the first. This happens because
of the second F<T> in DEFMINMAXSPEC.

* cummin/cummax don't return sparse matrices when called with sparse inputs.
The patches include Sparse[Complex]Matrix::cummin/cummax methods whose
complexity only depends on the number of positions with nonzero values (in the
input or output matrix) if the index is not requested.

* subsasgn cannot be used on a float complex scalar if the rhs is a double
scalar, a complex scalar or an empty matrix. I was incidentally trying to do
this for a corner case. For example:

s = single (3);    s(1) = 4  % float scalar = double scalar => ok
s = 2+3i;          s(1) = 4  % complex scalar = double scalar => ok
s = single (2+3i); s(1) = 4  % float complex scalar = double scalar => error

s(false) = zeros(0, 1) % error if s is a float complex scalar; works for other
scalar types


* In max.cc, the dimension test for max mistakenly calls min instead. Also,
this test:

%! x = [1, 2, 3, 4];  y = fliplr (x);
%! assert (min (x, 2i), [2i 2i 3 4]);

should expect: [1 2 2i 2i].

By using the attached random test generators, I tested the two solutions both
one vs. another and for sparse matrices vs. full 2D arrays.

Please let me know if you have any questions.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?50007>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/