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

[Gawade P]

Thanks, Even. Sure, I will run the full test suite, prepare the patch,
raise a ticket and update it.

On Fri, Aug 28, 2015 at 12:52 PM, Even Rouault
<even.rouault at spatialys.com> wrote:
> 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