3:15 a.m.
Hi,
indirectly prompted by Artur's feature request (...) I stumbled over the
following behavior, which looks like a bug to me:
<hansl>
M = I(2)
M[2,1] = NA
print M
eval imaxr(M) # wrong: {1;1}
eval imaxc(M) # correct
</hansl>
That is, the NA doesn't lead to an NA result (which is fine even though
I'm not sure it's intended), but for imaxr the result picks the NA entry.
This is with a recent snapshot.
thanks
sven
8:35 a.m.
On Mon, Jun 27, 2022 at 3:15 AM Sven Schreiber <svetosch(a)gmx.net> wrote:
Hi,
indirectly prompted by Artur's feature request (...) I stumbled over the
following behavior, which looks like a bug to me:
<hansl>
M = I(2)
M[2,1] = NA
print M
eval imaxr(M) # wrong: {1;1}
eval imaxc(M) # correct
</hansl>
That is, the NA doesn't lead to an NA result (which is fine even though
I'm not sure it's intended), but for imaxr the result picks the NA entry.
These functions were operating on the assumption that matrices cannot
contain NAs. For example, with imaxr() we were provisionally taking
the first element in a row as greatest, and changing that result only
if we found an element that compared greater than the first --
something that will never happen if the first element is NA.
I think the right thing to do is set the result to NA if an NA value
is encountered in the row or column in question. That's now in git.
Allin
9:31 a.m.
Am 27.06.2022 um 14:35 schrieb Cottrell, Allin:
I think the right thing to do is set the result to NA if an NA value
is encountered in the row or column in question. That's now in git.
Thanks, Allin, that handling is certainly consistent with the behavior
of similar such functions in gretl.
cheers
sven
9:41 a.m.
Am 27.06.2022 um 15:31 schrieb Sven Schreiber:
Am 27.06.2022 um 14:35 schrieb Cottrell, Allin:
>
> I think the right thing to do is set the result to NA if an NA value
> is encountered in the row or column in question. That's now in git.
Thanks, Allin, that handling is certainly consistent with the behavior
of similar such functions in gretl.
P.S.: I don't know how relevant this is, but this bugfix would change
the implicit behavior of these functions also in cases where the result
wasn't really wrong. What I mean is: If I understand you correctly, all
NAs except the (potential) one in the first element were implicitly
being ignored. This is a reasonable behavior, even if it was unintended
-- such behavior is what Artur's feature request was about.
So if people were using the functions intentionally like that, their
code will now be broken. I'm not saying the bugfix is wrong, but maybe
we should think about how to deal with (or even detect such) possible
fallout.
thanks
sven
1:18 p.m.
Am 27.06.22 um 15:41 schrieb Sven Schreiber:
Am 27.06.2022 um 15:31 schrieb Sven Schreiber:
> Am 27.06.2022 um 14:35 schrieb Cottrell, Allin:
>>
>> I think the right thing to do is set the result to NA if an NA value
>> is encountered in the row or column in question. That's now in git.
>
> Thanks, Allin, that handling is certainly consistent with the behavior
> of similar such functions in gretl.
>
P.S.: I don't know how relevant this is, but this bugfix would change
the implicit behavior of these functions also in cases where the result
wasn't really wrong. What I mean is: If I understand you correctly, all
NAs except the (potential) one in the first element were implicitly
being ignored. This is a reasonable behavior, even if it was unintended
-- such behavior is what Artur's feature request was about.
So if people were using the functions intentionally like that, their
code will now be broken. I'm not saying the bugfix is wrong, but maybe
we should think about how to deal with (or even detect such) possible
fallout.
The minimum is to inform people via the mailing list that the behavior
of some built-in function(s) has changed - I would say.
_Every_ software project has bugs, and this just shows how crucial it is
to write tests when developing code. At the very end everyone is
responsible for his/ her own code and has, in the best case, written
tests to detect bugs or changes due to changed versions.
On the gretl developer side, we should try to do as best as possible to
avoid, detect and fix bugs. Luckily, we already have a suit of tests.
But, as this case has shown, we should also have some for apparently
trivial functions such as imaxr().
And to mention this: Interested people of the community can help us to
extend our already existing but non-exhaustive battery of tests ;-)
Please contact us, if you want to help us.
Artur
1:55 p.m.
On Mon, Jun 27, 2022 at 9:41 AM Sven Schreiber <svetosch(a)gmx.net> wrote:
Am 27.06.2022 um 15:31 schrieb Sven Schreiber:
> Am 27.06.2022 um 14:35 schrieb Cottrell, Allin:
>>
>> I think the right thing to do is set the result to NA if an NA value
>> is encountered in the row or column in question. That's now in git.
>
> Thanks, Allin, that handling is certainly consistent with the behavior
> of similar such functions in gretl.
>
P.S.: I don't know how relevant this is, but this bugfix would change
the implicit behavior of these functions also in cases where the result
wasn't really wrong. What I mean is: If I understand you correctly, all
NAs except the (potential) one in the first element were implicitly
being ignored.
Yes, that's what was going on -- and it was clearly untenable. There's
no reason to treat NAs (NaNs) differently if they happen to fall in
the first element of a row or column versus later on.
Any fix is going to change the behavior of these functions, in the
pathological case where matrices contain NaNs. I'd rather insist on
propagation of NaNs according to IEEE rules than trying to concoct a
way of ignoring them. Maybe it's worth noting that if you're doing
something truly "matrix-y" with matrices (multiplication,
decomposition, inversion or whatever) there's absolutely no way to
ignore NaNs, and it's redundant to say "note that we're not ignoring
NaNs".
Allin
874
days inactive
874
days old
5 comments
3 participants
participants (3)
-
Artur T. -
Cottrell, Allin -
Sven Schreiber