[Qgis-developer] Provider native raster band stats

Fri Aug 5 10:43:18 EDT 2011

Regarding the progress function, I have found a much simpler and elegant solution:  

- call the function GDALGetRasterStatistics(...,bForce=FALSE,...) to get the cached stats (and not force a calculation)

- if the stats are not found, then call GDALComputeRasterStatistics() which supports the progress callback

No need to modify anything in GDAL!  Guess I was braindead a few days ago...

I'll send the code to Tim so he can update the trunk.

Etienne

________________________________
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: Thursday, August 4, 2011 4:40:52 AM
Subject: Re: [Qgis-developer] Provider native raster band stats

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
==============================================