Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test comparisons between scalars and 0-dim PDLs with badflag == 1 #128

Merged
merged 10 commits into from Jul 28, 2015

Conversation

zmughal
Copy link
Member

@zmughal zmughal commented Jun 21, 2015

This is to elucidate the issues with stat() and per-PDL badvalues
brought up by Marek Gierliński in SF#390
http://sourceforge.net/p/pdl/bugs/390/,
#124.

@zmughal
Copy link
Member Author

zmughal commented Jul 7, 2015

@zmughal
Copy link
Member Author

zmughal commented Jul 18, 2015

Plan for further work is discussed at http://irclog.perlgeek.de/pdl/2015-07-18#i_10918794:

  • create a warning for when a comparison op has *_badval ∈ {0, 1} ⇒ merge to master & do a full release
  • change the output type for comparisons to byte ⇒ test on devel branch, see what breaks, & do a dev release

@mohawk2
Copy link
Member

mohawk2 commented Jul 26, 2015

Why not just unconditionally delete $extra{Exception}; (lines 92-6 of ops.pd)? Similarly, lines 248-54.

I'm also keen to squash together / eliminate failing commits, to ease future bisecting.

The bit of primitive.pd you changed looks like it's crying out for a bit of data-driven action.

This is to elucidate the issues with stat() and per-PDL badvalues
brought up by Marek Gierliński in SF#390
<http://sourceforge.net/p/pdl/bugs/390/>,
<#124>.

When testing the scalar comparison, test across several values on either
side of bad value so that it is clear which case is failing.

The test must be skipped if BADVAL_USENAN: the test sets `badvalue()`.
Also added a note that the parameter is unused because it is not
involved in setting `$badcode` for exceptional cases for each function
as intended (e.g., negative values for `sqrt()` would require complex
numbers to be returned).

This helps move PDL towards using `use strict` everywhere.
When checking for badvalues in binary operators, we need to be sure that
each of the operands are not only badvalues, but also **have the badflag
set**. This way non-bad PDLs and scalars do not get treated as bad when
they are not.
The documentation for `statsover()` states that it ignores badvalues.
Since `stats()` calls `statsover()`, this behaviour applies there as
well.
Converted the older tests to be data-driven.
When using a comparison operator, inconsistent results can appear when a
badvalue is either 0 or 1. This is because {0, 1} is the range of all
operators that return a logical PDL. This means that if the input PDL
contains badvalues, it can not both represent badvalues that are the
result of computing the operator *and* logical values at the same time.

This is a continuation of the work addressing the problem brought up in
<http://sourceforge.net/p/pdl/bugs/390/>, <#124>.
@zmughal
Copy link
Member Author

zmughal commented Jul 28, 2015

I squashed and reordered the commits.

I want to hold off on the two things you mentioned:

  • turning the badvalue propagation section in statsover() into a loop that applies the same to each return value: this would not give that much reduction in complexity. Not enough to justify having a a for/map to generate code.
  • removing the Exception key in ops.pd: the Exception is actually a placeholder for more sane ops...it is just that it was never implemented properly. There's a bug there and I'd rather leave it in for now as a greppable reminder.

zmughal added a commit that referenced this pull request Jul 28, 2015
test comparisons between scalars and 0-dim PDLs with badflag == 1
@zmughal zmughal merged commit cd52aae into master Jul 28, 2015
@zmughal zmughal modified the milestone: PDL v2.012_01 Jul 29, 2015
@zmughal zmughal deleted the sf390 branch August 5, 2015 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants