[gdal-dev] Issue with gdalchksum on Fedora 21 ppc64le?

[Even Rouault]

Gawade,

Looks good to me. Could you prepare a patch for the changes and attach it to a 
ticket ? Thanks

Even

> Hi Even,
> 
> Please find my response below:
> >Interesting, I wouldn't have thought this to change the result of the
> >rounding, but who knows... Could you compare the color tables to check
> >that the differences are just within a +/- 1 amplitude ?
> 
> Yes. The map does not change for +ve slope values. For -ve slope
> values there is a consistent deviation of -1 through all values
> Attaching a sample data set for your reference.
> 
> >Yes, once we get same results on both platforms, adjusting the expected
> >checksums of nwt_grd.py / nwt_grc.py scripts might be needed.
> 
> Confirmed that we are getting the same results on both platforms.
> Looks like no changes are needed in nwt_grc.py. It still passes and
> checksum value is still same.
> Updated nwt_grd.py as follows, and it passes:
>     tst2 = gdaltest.GDALTest( 'NWT_GRD', 'nwt_grd.grd', 2, 33690 )
>     status2 = tst2.testOpen()
>     tst3 = gdaltest.GDALTest( 'NWT_GRD', 'nwt_grd.grd', 3, 20365 )
>     status3 = tst3.testOpen()
> 
> Please let me know if any more verification is required. Thanks!
> 
> 
> On Thu, Aug 27, 2015 at 4:00 PM, Even Rouault
> 
> <even.rouault at spatialys.com> wrote:
> > Le jeudi 27 août 2015 12:23:07, Gawade P a écrit :
> >> Thanks, Even.I have tried this change on ppc and there were some
> >> differences in the map values resulting in a small deviation in the
> >> final checksum value compared to the current output on x86
> >> The output on ppc with this change is:
> >> bash-4.2# gdalchksum.py nwt_grd.grd
> >> 28093
> >> 33690
> >> 20365
> >> 25856
> >> The expected values for band 2 and 3 are :
> >> 33626
> >> 20260
> > 
> > Interesting, I wouldn't have thought this to change the result of the
> > rounding, but who knows... Could you compare the color tables to check
> > that the differences are just within a +/- 1 amplitude ?
> > 
> > Thinking more, a cause for a difference of rounding should happen with
> > negative values, since casting truncates to the upper integer in case of
> > negative value. I'd think my proposed formula to be (slightly) more
> > accurate in case of negative slope, since it will only cast positive
> > values.
> > 
> >> I have not tried this change on x86, will try and let you know. Could
> >> there be any implications of these changed values elsewhere in the
> >> code (Assuming that they will change on x86 as well)? I believe some
> >> test cases might fail as well...
> > 
> > Yes, once we get same results on both platforms, adjusting the expected
> > checksums of nwt_grd.py / nwt_grc.py scripts might be needed.
> > 
> >> On Thu, Aug 27, 2015 at 3:10 PM, Even Rouault
> >> 
> >> <even.rouault at spatialys.com> wrote:
> >> > Le jeudi 27 août 2015 06:20:41, Gawade P a écrit :
> >> >> Good Morning, Even.
> >> >> I debugged this more, focusing, as you suggested, on the  computation
> >> >> of the colormap and looks like the difference in the value of the
> >> >> checksum is due to the difference in calculation of the color map
> >> >> values for negative slopes. The following lines of code in
> >> >> frmts/northwood/northwood.cpp, createIP() function are exhibiting
> >> >> 
> >> >> different behavior on intel and ppc:
> >> >>      for( i = wm + 1; i < index; i++)
> >> >>     
> >> >>     {
> >> >>     
> >> >>         map[i].r = map[wm].r + (unsigned char)(((i - wm) * rslope) +
> >> >>         0.5); map[i].g = map[wm].g + (unsigned char)(((i - wm) *
> >> >>         gslope) + 0.5); map[i].b = map[wm].b + (unsigned char)(((i -
> >> >>         wm) * bslope) + 0.5);
> >> >>     
> >> >>     }
> >> >> 
> >> >> primarily, because of the (unsigned char) casting of the second
> >> >> operand. Looks like the behavior could be undefined if the float
> >> >> value resulting from the computation of the expression (((i - wm) *
> >> >> bslope) + 0.5) is negative, as would be the case for negative slope.
> >> >> For ppc, the expression  (unsigned char)(((i - wm) * bslope) + 0.5)
> >> >> is evaluating to zero, whenever the result of (((i - wm) * bslope) +
> >> >> 0.5) is -ve i.e. for -ve slope
> >> > 
> >> > Interesting. It is likely that the result of the cast (unsigned
> >> > char)negative_floating_point_value is undefined by the C standard and
> >> > there might be differences in behaviour according to the CPU here
> >> > (seems to be confirmed by
> >> > http://stackoverflow.com/questions/2490600/iphone-floats-cast-to-
> >> > unsigned-ints-get-set-to-0-if-they-are-negative)
> >> > 
> >> > I think the following should be portable:
> >> > 
> >> > map[i].r = (unsigned char)(map[wm].r + (i - wm) * rslope + 0.5);
> >> > map[i].g = (unsigned char)(map[wm].g + (i - wm) * gslope + 0.5);
> >> > map[i].b = (unsigned char)(map[wm].b + (i - wm) * bslope + 0.5);
> >> > 
> >> > Could you confirm ?
> >> > 
> >> > W might want to explictly clamp to [0,255] before casting, but
> >> > according to the formulas of the slope, I'm confident the result
> >> > should already be in [0,255.X] range.
> >> > 
> >> > Even
> >> > 
> >> > --
> >> > Spatialys - Geospatial professional services
> >> > http://www.spatialys.com
> > 
> > --
> > Spatialys - Geospatial professional services
> > http://www.spatialys.com

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com