[Qgis-developer] Provider native raster band stats

Thu Aug 4 06:29:36 EDT 2011

Hi

For all interested, I have applied the first changes to master (see
http://linfiniti.com/2011/08/improvements-to-raster-performance-in-qgis-master/)....and
with a unit test! :-)

I still need to apply Etienne's patch for progress bar support which I
hope to do in this week.

regards

Tim

On Thu, Aug 4, 2011 at 9:40 AM, Tim Sutton <lists at linfiniti.com> wrote:
> Hi
>
> On Wed, Aug 3, 2011 at 7:38 PM, Etienne <etiennesky at yahoo.com> wrote:
>> Hi, I'm glad I can help!
>>
>> 1) regarding the histogram caching:
>> GDAL does this automatically, it looks for existing aux file and a
>> suitable histogram block.  It's all in function
>> GDALPamRasterBand::PamFindMatchingHistogram() in file
>> gdalpamrasterband.cpp .
>> As long as the dataset has support for PAM (stored in the .aux.xml file), it should work.
>> I found a bug in the floating-point comparison which was already fixed: http://trac.osgeo.org/gdal/ticket/4183
>>
>> Unlike the stats functions (GetStatistics/ComputeStatistics), there is only 1 function which looks for cached data.
>>
>>
>
> Ok for me it seems to recompute the histogram every time (which takes
> about a minute+ with the sample data set I sent you). I will update my
> gdal svn copy to get your patch for #4183 and see if the problem goes
> away.
>
>> 2) regarding the caching of stats:
>> Actually the GDAL driver looks for stored metadata (in function  GDALRasterBand::GetStatistics).
>> This works for gtiff files, I don't know for other raster types.
>>
>
> ok
>
>>
>> 3) regarding the progress bar:
>> Do you have sufficient authority to ask the GDAL folks to change the prototype for GDALGetRasterStatistics() and GetStatistics() for them to include a progress call-back?  I could make a patch for it, but I don't know if they would accept it.
>> In the mean time, I can copy the GDAL code into  qgsgdalprovider and submit that patch to you.
>>
>
> Ok Martin addressed this in his email in this thread. I basically have
> no more authority than anyone else in the community - they evauate
> each request based on merit. As Martin mentions, breaking ABI
> compatibility will not go down well with them so providing a wrapper
> function which adds progress support (or perhaps adding the progress
> callback as the last param with a default of void so that it can be
> left out will be acceptable to them) would be good.
>
>> 4)
>> There is another issue I forgot to mention, and that is the bin count of the
>> histogram.  GDAL uses 256 bins, and recently the GDAL histogram
>> calculation  in QgsRasterLayerProperties::refreshHistogram()  has been
>> changed to use 255 bins. ( const int BINCOUNT = 255; ).  Could this be
>> put back to 256 for consistency with GDAL, or is there a reason for
>> that?
>>
>
> Ok I have changed in my local copy and will commit soon.
>
>>
>> Thanks! Etienne
>>
>
> Thank you!
>
> Regards
>
> Tim
>
>> ________________________________
>> From: Tim Sutton <lists at linfiniti.com>
>> To: Etienne <etiennesky at yahoo.com>
>> Cc: "Qgis-developer at lists.osgeo.org" <Qgis-developer at lists.osgeo.org>
>> Sent: Wednesday, August 3, 2011 6:00:56 AM
>> Subject: Re: [Qgis-developer] Provider native raster band stats
>>
>> Hi Again
>>
>> By the way is there a handy way to get the histogram from the aux.xml
>> too? I see it is stored there but haven't figured out how to retrieve
>> it from cache again.
>>
>>
>>
>>
>> ----- Original Message -----
>> From: Tim Sutton <lists at linfiniti.com>
>> To: Etienne <etiennesky at yahoo.com>
>> Cc: "Qgis-developer at lists.osgeo.org" <Qgis-developer at lists.osgeo.org>
>> Sent: Wednesday, August 3, 2011 6:00:56 AM
>> Subject: Re: [Qgis-developer] Provider native raster band stats
>>
>> Hi Again
>>
>> By the way is there a handy way to get the histogram from the aux.xml
>> too? I see it is stored there but havent figured out how to retrieve
>> it from cache again.
>>
>> Regards
>>
>> Tim
>>
>> On Wed, Aug 3, 2011 at 10:16 AM, Tim Sutton <lists at linfiniti.com> wrote:
>>> Hi
>>>
>>> On Tue, Aug 2, 2011 at 7:06 PM, Etienne <etiennesky at yahoo.com> wrote:
>>>> Tim, my impression is that it speeds things up, but I have a suggestion to make it faster.
>>>>
>>> Cool!
>>>
>>>> From what I have seen in your code, the  QgsGdalProvider::bandStatistics function calls GDALComputeRasterStatistics (), which computes the statistics every time - it doesn't fetch pre-calculated data ( which is stored in gtiff metadata, or aux.xml file).
>>>>
>>>> A better way would be to use the GDALGetRasterStatistics() which calls GDALComputeRasterStatistics() when needed (when bForce = TRUE)
>>>> double myerval =  GDALGetRasterStatistics ( myGdalBand, bApproxOK, TRUE, &pdfMin, &pdfMax, &pdfMean, &pdfStdDev);
>>>>
>>>
>>> Ah - actually I thought GDALComputeRasterStatistics was using aux.xml
>>> cached stats already. I guess I never read the docs carefully enough.
>>> I applied your change and my ~8min to load raster from my original
>>> email took about a minute to load the first time (while gdal was
>>> creating an aux.xml) and subsequent use of the same raster resulted in
>>> it opening instantly! So thanks for your excellent advice. I have
>>> pushed an updated version to my repo that incorporates your changes
>>> should others like to try. I will try to write some unit tests this
>>> week and then merge my branch into QGIS master over the weekend.
>>>
>>>>
>>>>
>>>> I have tested it on a few rasters, it gets the same stats.
>>>>
>>>> The downside is that GDALGetRasterStatistics() does not offer a progress callback, rendering the progressbar useless.
>>>> If it"s really needed, I could re-implement the function in  QgsGdalProvider (about 50 lines of code), or try to have GDALGetRasterStatistics support a progress callback.
>>>>
>>>
>>> Yes this is quite noticeable the first time the raster is loaded and
>>> you sit wondering what is happening. I guess the more elegant would be
>>> to have the change carried out in gdal itself if that is acceptible,
>>> otherwise your patch as per your former suggestion would be greatly
>>> appreciated.
>>>
>>> Best regards
>>>
>>> Tim
>>>
>>>> regards, Etienne
>>>>
>>>> ----- Original Message -----
>>>> From: Tim Sutton <lists at linfiniti.com>
>>>> To: Marco Hugentobler <marco.hugentobler at sourcepole.ch>
>>>> Cc: qgis-developer at lists.osgeo.org
>>>> Sent: Monday, July 11, 2011 7:04:08 PM
>>>> Subject: Re: [Qgis-developer] Provider native raster band stats
>>>>
>>>> Hi
>>>>
>>>>
>>>> On Mon, Jul 11, 2011 at 5:29 PM, Marco Hugentobler
>>>> <marco.hugentobler at sourcepole.ch> wrote:
>>>>> Hi Tim
>>>>>
>>>>> I'm not a raster guru, but the changes look good to me.
>>>>> +1 for merging.
>>>>
>>>> Thanks for your feedback Marco. I guess I'll write some unit tests
>>>> (wrong way around I know :-) ) and then merge it if they all pass and
>>>> there are no other objections. :-)
>>>>
>>>> Regards
>>>>
>>>> Tim
>>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> Marco
>>>>>
>>>>> Am Donnerstag, 7. Juli 2011, 00.47:25 schrieb Tim Sutton:
>>>>>> Hi
>>>>>>
>>>>>> During the Lisbon hackfest (between all those meetings!) I worked on
>>>>>> updating the raster providers to be able to generate stats themselves.
>>>>>> The raster provider base class has a default (and historically
>>>>>> inefficient as we all know) implementation for collecting stats by
>>>>>> walking over every cell in the raster twice. This method remains, but
>>>>>> the providers can now supply stats themselves (heopfully using more
>>>>>> efficient mechanisms). I have implemented gdal native stats already.
>>>>>> Over the last few weeks I did some further testing and cleanups to
>>>>>> this work. From my testing some of my worst case files opened in
>>>>>> ~1minute rather than ~8+ minutes. It would be great if interested
>>>>>> parties could test (details below) before I put it into master.
>>>>>>
>>>>>> git remote add timlinux git://github.com/timlinux/Quantum-GIS.git
>>>>>> git fetch timlinux
>>>>>> git branch --track timlinux raster-stats
>>>>>> git checkout raster-stats
>>>>>>
>>>>>> I look forward to your feedback.
>>>>>>
>>>>>> Regards
>>>>>
>>>>>
>>>>> --
>>>>> Dr. Marco Hugentobler
>>>>> Sourcepole -  Linux & Open Source Solutions
>>>>> Churerstrasse 22, CH-8808 Pfäffikon SZ, Switzerland
>>>>> marco.hugentobler at sourcepole.ch http://www.sourcepole.ch
>>>>> Technical Advisor QGIS Project Steering Committee
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Tim Sutton - QGIS Project Steering Committee Member (Release  Manager)
>>>> ==============================================
>>>> Please do not email me off-list with technical
>>>> support questions. Using the lists will gain
>>>> more exposure for your issues and the knowledge
>>>> surrounding your issue will be shared with all.
>>>>
>>>> Visit http://linfiniti.com to find out about:
>>>>  * QGIS programming and support services
>>>>  * Mapserver and PostGIS based hosting plans
>>>>  * FOSS Consulting Services
>>>> Skype: timlinux
>>>> Irc: timlinux on #qgis at freenode.net
>>>> ==============================================
>>>> _______________________________________________
>>>> Qgis-developer mailing list
>>>> Qgis-developer at lists.osgeo.org
>>>> http://lists.osgeo.org/mailman/listinfo/qgis-developer
>>>>
>>>> _______________________________________________
>>>> Qgis-developer mailing list
>>>> Qgis-developer at lists.osgeo.org
>>>> http://lists.osgeo.org/mailman/listinfo/qgis-developer
>>>>
>>>
>>>
>>>
>>> --
>>> Tim Sutton - QGIS Project Steering Committee Member (Release  Manager)
>>> ==============================================
>>> Please do not email me off-list with technical
>>> support questions. Using the lists will gain
>>> more exposure for your issues and the knowledge
>>> surrounding your issue will be shared with all.
>>>
>>> Visit http://linfiniti.com to find out about:
>>>  * QGIS programming and support services
>>>  * Mapserver and PostGIS based hosting plans
>>>  * FOSS Consulting Services
>>> Skype: timlinux
>>> Irc: timlinux on #qgis at freenode.net
>>> ==============================================
>>>
>>
>>
>>
>> --
>> Tim Sutton - QGIS Project Steering Committee Member (Release  Manager)
>> ==============================================
>> Please do not email me off-list with technical
>> support questions. Using the lists will gain
>> more exposure for your issues and the knowledge
>> surrounding your issue will be shared with all.
>>
>> Visit http://linfiniti.com to find out about:
>>  * QGIS programming and support services
>>  * Mapserver and PostGIS based hosting plans
>>  * FOSS Consulting Services
>> Skype: timlinux
>> Irc: timlinux on #qgis at freenode.net
>> ==============================================
>>
>>
>
>
>
> --
> Tim Sutton - QGIS Project Steering Committee Member (Release  Manager)
> ==============================================
> Please do not email me off-list with technical
> support questions. Using the lists will gain
> more exposure for your issues and the knowledge
> surrounding your issue will be shared with all.
>
> Visit http://linfiniti.com to find out about:
>  * QGIS programming and support services
>  * Mapserver and PostGIS based hosting plans
>  * FOSS Consulting Services
> Skype: timlinux
> Irc: timlinux on #qgis at freenode.net
> ==============================================
>

-- 
Tim Sutton - QGIS Project Steering Committee Member (Release  Manager)
==============================================
Please do not email me off-list with technical
support questions. Using the lists will gain
more exposure for your issues and the knowledge
surrounding your issue will be shared with all.

Visit http://linfiniti.com to find out about:
 * QGIS programming and support services
 * Mapserver and PostGIS based hosting plans
 * FOSS Consulting Services
Skype: timlinux
Irc: timlinux on #qgis at freenode.net
==============================================