orbisgis / geoclimate

Geospatial processing toolbox for environmental and climate studies
GNU Lesser General Public License v3.0
59 stars 15 forks source link

Wrong building table used by grid indicators ? #884

Closed j3r3m1 closed 9 months ago

j3r3m1 commented 9 months ago

It seems the grid indicator calculation is using buildings without the estimated height... It has to be checked. Might be a cache issue ?

j3r3m1 commented 9 months ago

OK, indeed there is a major bug in the OSM workflow. The building table containing the updated height is not used by the grid indicator nor the the noise indicator !!

@ebocher I update the code and you validate ?

I wonder whether it would also be an opportunity to move some parts of the OSM workflow into a more generic functions (in the workflow indicator) instead of having to maintain OSM and BDT workflows. This is particularly the case for the future: if we have a script which makes the indicator calculation for any input data it would be easier if everything is generic (outside the data related workflows).

j3r3m1 commented 9 months ago

OK, indeed there is a major bug in the OSM workflow. The building table containing the updated height is not used by the grid indicator nor the the noise indicator !!

In the osm_processing() in the WorkflowOSM.groovy file, the "buildingTableName" variable is sometimes used while it is updated within the code if the building height estimation is used. Thus "results.building" should be used instead (as it is the case in the "buildingPopulation" function).

j3r3m1 commented 9 months ago

@MelPoupelin do not use the grid indicators as long as this problem is not solved

ebocher commented 9 months ago

A mistake certainly due to the numerous reorganizations of the code in recent weeks. I suggest fixing the table building problem first, before starting a major restructuring that may cause new problems.

ebocher commented 9 months ago

I will send a PR to fix that. You've just rescued GeoClimate ;-)

ebocher commented 9 months ago

Fix in #890