[QGIS-Developer] Updating qgis:rastercalculator to work better in models

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[QGIS-Developer] Updating qgis:rastercalculator to work better in models

Rudi von Staden
Hi all,

I'm trying to make some updates to qgis:rastercalculator in processing
to make it work better in models. The raster calculator evaluates
expressions based on the source file name. The problem with using this
in the model builder is that the file names are not always known up
front.

The list of layers in the expression widget are of class
QgsProcessingModelChildParameterSource, so I'm trying to figure out
how to set an appropriate source name for different situations.

1. The layer is an input parameter
In this case there's no way to know the source name up front, but it
could be approximated by lyr.parameterName(). This assumes that the
parameterName and the source name of the layer are the same. It's a
bit of a stretch, but I can't think of a better way to do it.

2. The layer is an anonymous layer output by another algorithm
In this case the name of the output file can be known for certain from
lyr.outputName(). This adds another problem though, since most
algorithms have the same outputName ('OUTPUT'). I have worked around
this by adding an incrementing number to the end of the index (i.e.
OUTPUT, OUTPUT1, OUTPUT2, etc.). It's not perfect but at least then
there would be a viable way to build expressions with multiple inputs.

3. The layer is a named output from another algorithm
If the algorithm has a named output, then the source file name would
be a predictable composite of lyr.outputChildId() and the 'Parameter
name'. I expected to get this from lyr.outputName() once set, but it
seems that outputName is not updated (it remains OUTPUT for most
algorithms). Is there some other way to get the 'Parameter name'
string from QgsProcessingModelChildParameterSource (or some other
way)?

Handling these three situations would improve the situation, but it's
still a bit clunky (and complicated for users to understand). Another
option would be to forego the source() names altogether, and use an a,
b, c notation like many of the other raster calculators do. The
challenge then is to have the layer identifiers in the expression
builder match the layer identifiers that are passed to
QgsRasterCalculator. One way to achieve this would be to have the
'Layers' list in the expression builder show only layers that are
selected as 'Reference layers'. That way, only the layers that are
passed to Raster Calculator as parameters could be included in the
expressions, and there can be a consistent referencing pattern whether
you are using it as a standalone algorithm or in a model. I'm just not
sure how to get the list of selected Reference layers from inside
ExpressionWidgetWrapper. If there's interest in reworking it in this
way and somebody can point me in the right direction I'd be happy to
give it a go.

Some background to this here: https://issues.qgis.org/issues/19302

Kind regards,
Rudi
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

ginetto
I'm working on this in the spare time... btw I'm not sure if this is a good work to do.
I face the problems described by Rudi, because gdal_calc has some limitations working with not homogeneous rasters... this doe not not affect the native raster calculalator. BTW raster calculator has a "old" and "not standard" syntax whilst gdal_calc can use all the numpy power (that's a really big difference)

adding operators in raster calculator need c++ coding, doing in gdal_calc is not necessary (numpy is enough)

I know that some power users are stuk to 2.18 for the lack r.calc in grass7 integration in qgis (that seems will be solved soon)

My question (poll) is... does it make sense to patch native raster calc instead of override gdal_calc limitation?
Make sense to solve raster calculator bug only if these bugs are generic and does not depend of the specific organization of the raster calculator GUI.

Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Fri, 29 Jun 2018 at 22:35, Rudi von Staden <[hidden email]> wrote:
Hi all,

I'm trying to make some updates to qgis:rastercalculator in processing
to make it work better in models. The raster calculator evaluates
expressions based on the source file name. The problem with using this
in the model builder is that the file names are not always known up
front.

The list of layers in the expression widget are of class
QgsProcessingModelChildParameterSource, so I'm trying to figure out
how to set an appropriate source name for different situations.

1. The layer is an input parameter
In this case there's no way to know the source name up front, but it
could be approximated by lyr.parameterName(). This assumes that the
parameterName and the source name of the layer are the same. It's a
bit of a stretch, but I can't think of a better way to do it.

2. The layer is an anonymous layer output by another algorithm
In this case the name of the output file can be known for certain from
lyr.outputName(). This adds another problem though, since most
algorithms have the same outputName ('OUTPUT'). I have worked around
this by adding an incrementing number to the end of the index (i.e.
OUTPUT, OUTPUT1, OUTPUT2, etc.). It's not perfect but at least then
there would be a viable way to build expressions with multiple inputs.

3. The layer is a named output from another algorithm
If the algorithm has a named output, then the source file name would
be a predictable composite of lyr.outputChildId() and the 'Parameter
name'. I expected to get this from lyr.outputName() once set, but it
seems that outputName is not updated (it remains OUTPUT for most
algorithms). Is there some other way to get the 'Parameter name'
string from QgsProcessingModelChildParameterSource (or some other
way)?

Handling these three situations would improve the situation, but it's
still a bit clunky (and complicated for users to understand). Another
option would be to forego the source() names altogether, and use an a,
b, c notation like many of the other raster calculators do. The
challenge then is to have the layer identifiers in the expression
builder match the layer identifiers that are passed to
QgsRasterCalculator. One way to achieve this would be to have the
'Layers' list in the expression builder show only layers that are
selected as 'Reference layers'. That way, only the layers that are
passed to Raster Calculator as parameters could be included in the
expressions, and there can be a consistent referencing pattern whether
you are using it as a standalone algorithm or in a model. I'm just not
sure how to get the list of selected Reference layers from inside
ExpressionWidgetWrapper. If there's interest in reworking it in this
way and somebody can point me in the right direction I'd be happy to
give it a go.

Some background to this here: https://issues.qgis.org/issues/19302

Kind regards,
Rudi
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

Nyall Dawson
In reply to this post by Rudi von Staden
On Sat, 30 Jun 2018 at 06:35, Rudi von Staden <[hidden email]> wrote:
>
> Hi all,
>
> I'm trying to make some updates to qgis:rastercalculator in processing
> to make it work better in models. The raster calculator evaluates
> expressions based on the source file name. The problem with using this
> in the model builder is that the file names are not always known up
> front.

This is indeed very tricky. Regular QGIS expressions are fine here,
because we can use expression contexts, variables and the like to
expose model parameters to the expression engine and support this
requirement. The raster expression engine however is very limited and
doesn't have any existing way to support this use case.

> Handling these three situations would improve the situation, but it's
> still a bit clunky (and complicated for users to understand). Another
> option would be to forego the source() names altogether, and use an a,
> b, c notation like many of the other raster calculators do.

I personally think this may be the best available approach, which
doesn't require complex changes to the raster expression engine.

Nyall
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

Rudi von Staden
On Tue, 03 Jul 2018 at 12:03, Nyall Dawson <[hidden email]> wrote:
> Handling these three situations would improve the situation, but it's
> still a bit clunky (and complicated for users to understand). Another
> option would be to forego the source() names altogether, and use an a,
> b, c notation like many of the other raster calculators do.

I personally think this may be the best available approach, which
doesn't require complex changes to the raster expression engine.

Is there a way to access the list of selected reference layers from within the `ExpressionWidgetWrapper`? I haven't found any way to do this yet, and if the list of layers in the widget is different from the layers that gets passed to `QgsRasterCalculator` (like if new parameters or algorithms get added to the model) it could lead to strange errors and user confusion. It might then be best to return `QLineEdit` for the modeler view, like it already is for `DIALOG_BATCH`?

Rudi


_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

Rudi von Staden
In reply to this post by ginetto
On Mon, 2 Jul 2018 at 18:30, Luigi Pirelli <[hidden email]> wrote:
> My question (poll) is... does it make sense to patch native raster calc instead of override gdal_calc limitation?
> Make sense to solve raster calculator bug only if these bugs are generic and does not depend of the specific organization of the raster calculator GUI.

I've also encountered the issues using gdal_calc with non-homogenous
layers (which is why I started looking at the raster calculator).
Another reason I would like to see the native calc working is because
it runs much faster than having to wrap third-party tools. I've found
gdal is generally not too bad, but my use-case is still to build a
model and loop through it hundreds of times, so every second counts.

My feeling is that it wouldn't take much work to get the raster
calculator at least usable in the modeler at its current level of
functionality. Improving gdal_calc or implementing more powerful
operators in native raster calculator can then be a secondary, more
involved process.

Rudi
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

ginetto
hmmm... working on your test data, btw IMHO raster calculator should be dropped in favor of a numpy based expressions... we should also consider the maintenance aspect for the future! raster calculator is not hard to maintain (harder is the interface to integrate in modeler that the calculator itself) but it's syntax is really poor.

I didn't investigate how we can import gdal code (that's a python code) into qgis... or better create a gdal/ogr interface directly to qgis data (kind of R dataframe access)

Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Tue, 3 Jul 2018 at 15:02, Rudi von Staden <[hidden email]> wrote:
On Mon, 2 Jul 2018 at 18:30, Luigi Pirelli <[hidden email]> wrote:
> My question (poll) is... does it make sense to patch native raster calc instead of override gdal_calc limitation?
> Make sense to solve raster calculator bug only if these bugs are generic and does not depend of the specific organization of the raster calculator GUI.

I've also encountered the issues using gdal_calc with non-homogenous
layers (which is why I started looking at the raster calculator).
Another reason I would like to see the native calc working is because
it runs much faster than having to wrap third-party tools. I've found
gdal is generally not too bad, but my use-case is still to build a
model and loop through it hundreds of times, so every second counts.

My feeling is that it wouldn't take much work to get the raster
calculator at least usable in the modeler at its current level of
functionality. Improving gdal_calc or implementing more powerful
operators in native raster calculator can then be a secondary, more
involved process.

Rudi

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

Giovanni Manghi
In reply to this post by ginetto
Hi Luigi,

> I know that some power users are stuk to 2.18 for the lack r.calc in grass7
> integration in qgis (that seems will be solved soon)

what is greatly missing (in GRASS7) is r.mapcalculator.py, a script
made on top of r.mapcalc that among the other things allowed us to
have in Processing a nice way to use r.mapcalc (because it worked via
normal description files while r.mpalcalc does not, and any attempt to
make it work in Processing was not very succesful).

Are you saying that this script is being ported in GRASS7?

cheers

-- G --
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

ginetto
I was supposing is was subject to the GSOC about qgis/grass... but I can be wrong.
Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Tue, 3 Jul 2018 at 18:31, Giovanni Manghi <[hidden email]> wrote:
Hi Luigi,

> I know that some power users are stuk to 2.18 for the lack r.calc in grass7
> integration in qgis (that seems will be solved soon)

what is greatly missing (in GRASS7) is r.mapcalculator.py, a script
made on top of r.mapcalc that among the other things allowed us to
have in Processing a nice way to use r.mapcalc (because it worked via
normal description files while r.mpalcalc does not, and any attempt to
make it work in Processing was not very succesful).

Are you saying that this script is being ported in GRASS7?

cheers

-- G --
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

ginetto
Hi [hidden email] , [hidden email] and [hidden email] 

in the meantime I had to resolve compilation problems and OS update I'll show you what I'm doing to resurrect rastercalculator in modeler trying to evaluate if I'm doing well.

the principal problem (but not the only one) to run raster calculator in a modeler is to map input value to the formula. This is a generic problem that can be applied to any algorithm that need to map input in something internal to the algorithm.

The main problems is that the algorithm context scope hadn't information to allow this mapping. I added:

context.setExpressionContext(expContext);
here
and naming the createExpressionContextScopeForChildAlgorithm scope "algorithm_inputs" (actually the added scope does not have name, because was not necessary)

and add this mapping in ::variablesForChildAlgorithm
with value:
<var name>: <maplayer instance>
In this moment only <var name>_[extent] variable are added in this context scope

the idea, in the processAlgorithm python part, is to get the expression context from ProcessingContext and substitude <var name> with <maplayer source> in the formula that will be executed by QgsRasterCalculator

conceptually it works, but many other problems in context and parameter settings are present that seems they are more generic modeler problems I'm still trying to solve and reproduce them)

What I'm not sure is to use the expressionContext to store this information instead of a different or new contextScope. the advantages is that the new input parameters are available in the expression editor because available in the context.

as soon as I can I'll give you a PR where to talk about real code... in this moment I doing tests if my approach is (almost) correct and finding all limitation using these pathces in simple test models 

Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Wed, 4 Jul 2018 at 00:24, Luigi Pirelli <[hidden email]> wrote:
I was supposing is was subject to the GSOC about qgis/grass... but I can be wrong.
Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Tue, 3 Jul 2018 at 18:31, Giovanni Manghi <[hidden email]> wrote:
Hi Luigi,

> I know that some power users are stuk to 2.18 for the lack r.calc in grass7
> integration in qgis (that seems will be solved soon)

what is greatly missing (in GRASS7) is r.mapcalculator.py, a script
made on top of r.mapcalc that among the other things allowed us to
have in Processing a nice way to use r.mapcalc (because it worked via
normal description files while r.mpalcalc does not, and any attempt to
make it work in Processing was not very succesful).

Are you saying that this script is being ported in GRASS7?

cheers

-- G --
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Updating qgis:rastercalculator to work better in models

ginetto
please give a try to this preliminary PR that should fix the use of raster calculator in the modeler

https://github.com/qgis/QGIS/pull/7396

Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Mon, 9 Jul 2018 at 11:02, Luigi Pirelli <[hidden email]> wrote:
Hi [hidden email] , [hidden email] and [hidden email] 

in the meantime I had to resolve compilation problems and OS update I'll show you what I'm doing to resurrect rastercalculator in modeler trying to evaluate if I'm doing well.

the principal problem (but not the only one) to run raster calculator in a modeler is to map input value to the formula. This is a generic problem that can be applied to any algorithm that need to map input in something internal to the algorithm.

The main problems is that the algorithm context scope hadn't information to allow this mapping. I added:

context.setExpressionContext(expContext);
here
and naming the createExpressionContextScopeForChildAlgorithm scope "algorithm_inputs" (actually the added scope does not have name, because was not necessary)

and add this mapping in ::variablesForChildAlgorithm
with value:
<var name>: <maplayer instance>
In this moment only <var name>_[extent] variable are added in this context scope

the idea, in the processAlgorithm python part, is to get the expression context from ProcessingContext and substitude <var name> with <maplayer source> in the formula that will be executed by QgsRasterCalculator

conceptually it works, but many other problems in context and parameter settings are present that seems they are more generic modeler problems I'm still trying to solve and reproduce them)

What I'm not sure is to use the expressionContext to store this information instead of a different or new contextScope. the advantages is that the new input parameters are available in the expression editor because available in the context.

as soon as I can I'll give you a PR where to talk about real code... in this moment I doing tests if my approach is (almost) correct and finding all limitation using these pathces in simple test models 

Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Wed, 4 Jul 2018 at 00:24, Luigi Pirelli <[hidden email]> wrote:
I was supposing is was subject to the GSOC about qgis/grass... but I can be wrong.
Luigi Pirelli

**************************************************************************************************
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
* Hire me: http://goo.gl/BYRQKg
**************************************************************************************************


On Tue, 3 Jul 2018 at 18:31, Giovanni Manghi <[hidden email]> wrote:
Hi Luigi,

> I know that some power users are stuk to 2.18 for the lack r.calc in grass7
> integration in qgis (that seems will be solved soon)

what is greatly missing (in GRASS7) is r.mapcalculator.py, a script
made on top of r.mapcalc that among the other things allowed us to
have in Processing a nice way to use r.mapcalc (because it worked via
normal description files while r.mpalcalc does not, and any attempt to
make it work in Processing was not very succesful).

Are you saying that this script is being ported in GRASS7?

cheers

-- G --
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer