Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[modeler] r.cost named outputs are not passed as input to following algs #36379

Closed
luipir opened this issue May 12, 2020 · 10 comments · Fixed by #37128
Closed

[modeler] r.cost named outputs are not passed as input to following algs #36379

luipir opened this issue May 12, 2020 · 10 comments · Fixed by #37128
Assignees
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Processing Relating to QGIS Processing framework or individual Processing algorithms

Comments

@luipir
Copy link
Contributor

luipir commented May 12, 2020

Describe the bug
In modeler if r.cost output is set with a named output (as in the attached model), the ouptut is not passed as input to the following alg.
If named ouput is removed, then the temporary ouput is correctly passed to the next alg.

How to Reproduce

  1. Run the attached alg => "Could not load source layer for INPUT_RASTER: invalid value"
    because INPUT_RASTER is empty:
    Running Reclassify by table [2/2]
    Input Parameters:
    { DATA_TYPE: 1, INPUT_RASTER: '', NODATA_FOR_MISSING: False, NO_DATA: -9999, OUTPUT: 'TEMPORARY_OUTPUT', RANGE_BOUNDARIES: 1, RASTER_BAND: 1, TABLE: [0,1000,0,1000,10000,1] }

  2. remove "cumulative_cost" out variable from model and run again successfully

QGIS and OS versions

QGIS version 3.13.0-Master QGIS code revision 1435076
Compiled against Qt 5.9.5 Running against Qt 5.9.5
Compiled against GDAL/OGR 2.2.3 Running against GDAL/OGR 2.2.3
Compiled against GEOS 3.7.1-CAPI-1.11.1 Running against GEOS 3.7.1-CAPI-1.11.1 27a5e771
Compiled against SQLite 3.22.0 Running against SQLite 3.22.0
PostgreSQL Client Version 12.2 (Ubuntu 12.2-2.pgdg18.04+1) SpatiaLite Version 4.3.0a
QWT Version 6.1.3 QScintilla2 Version 2.10.2
PROJ.4 Version 493
OS Version Ubuntu 18.04.4 LTS This copy of QGIS writes debugging output.
Active python plugins processing_whitebox; plugin_reloader; processing_taudem; quick_map_services; galiciasustentable; SpreadsheetLayers; firstaid; IPyConsole; qconsolidate; excel_sync; db_manager; processing; MetaSearch

Additional context
Need at last merge o #36371 before to allow use grass output algs in modeler

@luipir luipir added Processing Relating to QGIS Processing framework or individual Processing algorithms Bug Either a bug report, or a bug fix. Let's hope for the latter! labels May 12, 2020
@luipir
Copy link
Contributor Author

luipir commented May 12, 2020

test_r_cost.zip
I forgot to attach the example model

@luipir
Copy link
Contributor Author

luipir commented May 13, 2020

r.cost ouptut are set as: QgsProcessingParameterRasterDestination in r.cost.txt
Any available input is searched among QgsProcessingOutputDefinition types as in:
https://github.com/qgis/QGIS/blob/master/src/core/processing/models/qgsprocessingmodelalgorithm.cpp#L884
@nyalldawson @alexbruy What would be the best fix, add another elements in sources list or just derivate QgsProcessingParameter*Destination also from QgsProcessingOutputDefinition ?
Why a ParameterDestination is not also a OutputDefinition? I'm missing some step?

@nyalldawson
Copy link
Collaborator

This is an issue in the grass provider -- it's incorrectly just returning the parameter value for the OUTPUT parameter, when it should be returning the path to the file it actually generated.

@nyalldawson nyalldawson self-assigned this Jun 11, 2020
nyalldawson added a commit to nyalldawson/QGIS that referenced this issue Jun 11, 2020
locations as the algorithm results, don't just echo back the parameter
value

Otherwise we don't return the correct value for temporary file locations
or for file paths during model execution

Fixes qgis#36379
@luipir
Copy link
Contributor Author

luipir commented Jun 11, 2020

tnx

nyalldawson added a commit that referenced this issue Jun 11, 2020
locations as the algorithm results, don't just echo back the parameter
value

Otherwise we don't return the correct value for temporary file locations
or for file paths during model execution

Fixes #36379
@asierrl
Copy link

asierrl commented Nov 28, 2020

I found the same issue with r.null. When used within the graphical modeler, its output name is correctly pased to the GDAL raster calculator only if the output is not named. When I name the output the entry parameters for the GDAL:rastercalculator show an empty input A, as Input A: " ".
I'm working with a QGIS 3.16.0-1, GRASS 7.8.3-1 and GDAL library 3.1.4rc2-1, on windows 10 Education (64 bits), version 1909.

@gioman
Copy link
Contributor

gioman commented Nov 29, 2020

I found the same issue with r.null. When used within the graphical modeler, its output name is correctly pased to the GDAL raster calculator only if the output is not named. When I name the output the entry parameters for the GDAL:rastercalculator show an empty input A, as Input A: " ".
I'm working with a QGIS 3.16.0-1, GRASS 7.8.3-1 and GDAL library 3.1.4rc2-1, on windows 10 Education (64 bits), version 1909.

@asierrl attach the model

@asierrl
Copy link

asierrl commented Dec 1, 2020

Sorry, I didn't know even how to do it. Is a Zip file OK?
This is the final version, but the issue appeared at a middle point of the development, when the model was much simpler. And it happens with any of the two instances of r.null it contains.

ContarCalasProbaQGIS6.zip

@gioman
Copy link
Contributor

gioman commented Dec 1, 2020

This is the final version, but the issue appeared at a middle point of the development, when the model was much simpler. And it happens with any of the two instances of r.null it contains.

@asierrl can you kindly provide also some sample input data?

@asierrl
Copy link

asierrl commented Dec 1, 2020

Sure, @gioman, this is the trial dataset I've been using to test the model.

CalasClasif.zip

@gioman
Copy link
Contributor

gioman commented Dec 1, 2020

Sure, @gioman, this is the trial dataset I've been using to test the model.

CalasClasif.zip

confirmed on 3.16.1, should this be reopened? @luipir @nyalldawson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
4 participants