PDLPorters/pdl

setops gives erroneous results

jo-37 opened this issue · 2 comments

jo-37 commented

setops's intersection operation shows erroneous behaviour if one operand is a singleton (or a repeated single value) and the other operand is empty. The result is not empty as expected, but the singleton.

In addition to this error there is some strange behaviour if the other operand is a repeated value instead of empty. Here the result is not unique. (Discovered by @choroba)

Example code to reproduce:

#!/usr/bin/perl

use v5.16;
use warnings;
use PDL;

for my $rhs (zeroes(0), ones(2)) {
    for my $lhs (pdl(1), ones(1), ones(4), sequence(4)) {
        say "$lhs and $rhs: ", setops $lhs, 'AND', $rhs;
    }
}
__DATA__
1 and Empty[0]: [1]
[1] and Empty[0]: [1]
[1 1 1 1] and Empty[0]: [1]
[0 1 2 3] and Empty[0]: Empty[0]
1 and [1 1]: [1 1]
[1] and [1 1]: [1 1]
[1 1 1 1] and [1 1]: [1 1]
[0 1 2 3] and [1 1]: [1]

Thank you for the report! I've turned the above into this in t/primitive.t, which is more readable for me:

my @cases = (
  [ pdl(1), empty(), empty() ],
  [ ones(1), empty(), empty() ],
  [ ones(4), empty(), empty() ],
  [ sequence(4), empty(), empty() ],
  [ pdl(1), ones(2), ones(1) ],
  [ ones(1), ones(2), ones(1) ],
  [ ones(4), ones(2), ones(1) ],
  [ sequence(4), ones(2), ones(1) ],
);
ok tapprox(setops($_->[0], 'AND', $_->[1]), $_->[2]), "$_->[0] AND $_->[1]" for @cases;

The underlying cause for this is that uniq, despite being used in a number of places, has exactly no tests! (and evidently is slightly wrong)

In fact, uniq was fine, and it was just two bugs in the setops function. Thank you for reporting, and good find by @choroba!

This will be released in the imminent next version of PDL, once I am sure there are no other problems needing fixing for PDL::OpenCV.