From 015d9fe5c91f10c9a81c63bb086c413aeb304665 Mon Sep 17 00:00:00 2001 From: Zakariyya Mughal Date: Sun, 10 Jul 2016 16:18:35 -0500 Subject: [PATCH] Fixes badval propagation to support all badval configurations The changes in broke the build for all cases of `BADVAL_USENAN` xor `BADVAL_PER_PDL`. This was because the checking for badvalues when copying in `pdl_kludge_copy_*` did not take into account these variations in accessing and setting the badvalue. For example, a simple comparison using `==` is not valid when NaN values are being compared. Here we need to use `!finite(...)` to make the comparison if the type can use NaN values. --- Basic/Core/pdlcore.c.PL | 22 +++++++++++++++++++--- t/constructor.t | 4 ++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Basic/Core/pdlcore.c.PL b/Basic/Core/pdlcore.c.PL index 7c1e00006..b667adacf 100644 --- a/Basic/Core/pdlcore.c.PL +++ b/Basic/Core/pdlcore.c.PL @@ -1077,6 +1077,11 @@ PDL_Indx pdl_kludge_copy_$type(PDL_Indx poff, // Offset into the dest data array pdlsiz = source_pdl->dims[pdldim]; } +#if BADVAL + /* This is used inside the switch in order to detect badvalues. */ + PDL_Anyval source_badval = PDL.get_pdl_badvalue(source_pdl); +#endif /* BADVAL */ + /* This is the actual data-copying code. It is generated with a Perl loop, to * ensure that all current PDL types get treated. */ @@ -1084,12 +1089,23 @@ PDL_Indx pdl_kludge_copy_$type(PDL_Indx poff, // Offset into the dest data array !WITH!SUBS! # perl loop to emit code for all the PDL types -- ctype gets the C type of - # the source PDL, and switch_type gets the Perl name. + # the source PDL, switch_type gets the Perl name, ppsym gets + # the symbol need to retrieve from a PDL_Anyval, and type_usenan is a + # boolean indicating whether this type handles NaNs. foreach my $switch_type (keys %PDL::Types::typehash) { my $ctype = $PDL::Types::typehash{$switch_type}{ctype}; my $stype = $PDL::Types::typehash{$switch_type}{ctype}; $stype =~ s/PDL_//; + my $ppsym = $PDL::Types::typehash{$switch_type}{ppsym}; + my $type_usenan = $PDL::Types::typehash{$switch_type}{usenan}; + + my $comp_for_nan = + $type_usenan + # if not equal, check if both are NaN + ? "( !finite( (($ctype *)pptr)[i] ) && !finite(source_badval.value.$ppsym) )" + # otherwise it must be false + : '0'; print OUT <<"!WITH!SUBS!"; @@ -1100,10 +1116,10 @@ PDL_Indx pdl_kludge_copy_$type(PDL_Indx poff, // Offset into the dest data array for(; ihas_badvalue || (source_pdl->state & PDL_BADVAL)) { - if( (($ctype *)pptr)[i] == PDL.bvals.$stype ) { + /* Retrieve directly from .value.* instead of using ANYVAL_EQ_ANYVAL */ + if( (($ctype *)pptr)[i] == source_badval.value.$ppsym || $comp_for_nan ) { /* bad value in source PDL -- use our own type's bad value instead */ pdata[i] = PDL.bvals.$type; - p->has_badvalue=1; p->state |= PDL_BADVAL; } else { pdata[i] = (PDL_$type) ((${ctype} *)pptr)[i]; diff --git a/t/constructor.t b/t/constructor.t index 90e62baab..87c2848cd 100644 --- a/t/constructor.t +++ b/t/constructor.t @@ -129,14 +129,14 @@ is $p->at(1,0), $PDL::undefval, "scalar got padded OK"; is $p->at(0,1), $pdl_v->at(0), "vector element 0 got copied OK"; is $p->at(1,1), $pdl_v->at(1), "vector element 1 got copied OK"; -## A more complicated case +## A more complicated case $p = pdl($pdl_s, 5, $pdl_v, $pdl_m, [$pdl_v, $pdl_v]); isa_ok($p,'PDL'); is $p->ndims(), 3, 'complicated case -> 3-d PDL'; is $p->dim(0), 2, 'complicated case -> dim 0 is 2'; is $p->dim(1), 2, 'complicated case -> dim 1 is 2'; is $p->dim(2), 5, 'complicated case -> dim 1 is 5'; -@testvals = ([ [0,0,0], 2 ], [ [1,0,0], 0 ], [ [0,1,0], 0 ], [ [1,1,0], 0 ], +@testvals = ([ [0,0,0], 2 ], [ [1,0,0], 0 ], [ [0,1,0], 0 ], [ [1,1,0], 0 ], [ [0,0,1], 5 ], [ [1,0,1], 0 ], [ [0,1,1], 0 ], [ [1,1,1], 0 ], [ [0,0,2], 3 ], [ [1,0,2], 0 ], [ [0,1,2], 4 ], [ [1,1,2], 0 ], [ [0,0,3], 5 ], [ [1,0,3], 6 ], [ [0,1,3], 7 ], [ [1,1,3], 8 ],