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

[Gawade P]

Hi Even,

I have raised the ticket: https://trac.osgeo.org/gdal/ticket/6091
and attached the patch to it.

Thanks.

On Fri, Aug 28, 2015 at 2:48 PM, Gawade P <gawade.oefp at gmail.com> wrote:
> 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