[QGIS-Developer] What to do about incorrect SAGA algorithms?

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[QGIS-Developer] What to do about incorrect SAGA algorithms?

Nyall Dawson
Hi list,

Just raising the discussion about what we should do with SAGA
Processing algorithms when the results generated by SAGA are known to
be incorrect (i.e. there's a bug in SAGA itself).

Specifically, the SAGA difference and symmetric difference algorithms
generate incorrect results. See https://issues.qgis.org/issues/21354,
and the results from
https://github.com/qgis/QGIS/blob/master/python/plugins/processing/tests/testdata/saga_algorithm_tests.yaml#L318
. This seems to be a general problem with the algorithms, not isolated
to particular input data.

My gut feeling is: we should "deprecate" these algorithms (which means
they continue to work for existing models, but are hidden from the UI
and cannot be run from the toolbox or added to new models). I don't
see the value in exposing broken algorithms when we have robust
alternatives (the native QGIS difference/symmetric difference
algorithms are very fast, stable, and well tested), and potentially a
lot of harm.

Thoughts?

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: What to do about incorrect SAGA algorithms?

pcav
Hi Nyall,

On 04/03/19 12:11, Nyall Dawson wrote:

> Hi list,
>
> Just raising the discussion about what we should do with SAGA
> Processing algorithms when the results generated by SAGA are known to
> be incorrect (i.e. there's a bug in SAGA itself).
>
> Specifically, the SAGA difference and symmetric difference algorithms
> generate incorrect results. See https://issues.qgis.org/issues/21354,
> and the results from
> https://github.com/qgis/QGIS/blob/master/python/plugins/processing/tests/testdata/saga_algorithm_tests.yaml#L318
> . This seems to be a general problem with the algorithms, not isolated
> to particular input data.
>
> My gut feeling is: we should "deprecate" these algorithms (which means
> they continue to work for existing models, but are hidden from the UI
> and cannot be run from the toolbox or added to new models). I don't
> see the value in exposing broken algorithms when we have robust
> alternatives (the native QGIS difference/symmetric difference
> algorithms are very fast, stable, and well tested), and potentially a
> lot of harm.
>
> Thoughts?

thanks for raising this. I fully agree, and will push things a bit
further: IMHO we should hide all algs from other backends when we have a
robust alternative. This means qgis core alg should:
* cover the full range of options available in the alternative command
(often e.g. GRASS has a number of options that can be useful in special
cases)
* be robust even for huge data sets (e.g. GRASS is usually conservative
in the use of memory, and can complete analyses even when other algs fail)

* life is not perfect, so I think a power user should have the
possibility of using other algs, in case we overlooked a special option,
on temporarily if our alg is broken.

I therefore suggest to:
1. have a list of the potentially duplicated algs
2. find one or more volunteers checking thoroughly the above
3. hiding those who correspond to the criteria
4. design a last resort way of using hidden algs.
I'm available for 2.
All the best.
--
Paolo Cavallini - www.faunalia.eu
QGIS.ORG Chair:
http://planet.qgis.org/planet/user/28/tag/qgis%20board/
_______________________________________________
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: What to do about incorrect SAGA algorithms?

Giovanni Manghi
Hi Nyall,



Just raising the discussion about what we should do with SAGA
Processing algorithms when the results generated by SAGA are known to
be incorrect (i.e. there's a bug in SAGA itself).

add some kind of warning (for the tools we know can have issue), do not remove, please. After all we never removed (nor deprecated) the native QGIS tools when we knew (sometimes since long)  that they were spitting very wrong results...

just my 2 cents

-- Giovanni --



_______________________________________________
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: What to do about incorrect SAGA algorithms?

Pedro Venâncio-2
In reply to this post by Nyall Dawson
Hi Nyall,

I think this rises another problem, that is the use of a very old version of SAGA (2.3.2) in QGIS.

I've just tested Difference algorithm with Lene dataset in native SAGA 2.3.2 and 7.2.0, and 2.3.2 gives the problem described, but 7.2.0 gives the correct result:



So, we are faced again with the question of keep an old version of SAGA in QGIS core, or make it as an external plugin, with someone keeping an eye in the changes introduced by SAGA team between versions (as we saw in the past).

Alexander Bruy had made an effort to keep a parallel SAGA plugin that support newer SAGA versions: https://plugins.bruy.me/processing-saga.html
But it is not in the official repository. Maybe this is the time to discuss again the use of SAGA as an external plugin?

Best regards,
Pedro Venâncio




Nyall Dawson <[hidden email]> escreveu no dia segunda, 4/03/2019 à(s) 11:11:
Hi list,

Just raising the discussion about what we should do with SAGA
Processing algorithms when the results generated by SAGA are known to
be incorrect (i.e. there's a bug in SAGA itself).

Specifically, the SAGA difference and symmetric difference algorithms
generate incorrect results. See https://issues.qgis.org/issues/21354,
and the results from
https://github.com/qgis/QGIS/blob/master/python/plugins/processing/tests/testdata/saga_algorithm_tests.yaml#L318
. This seems to be a general problem with the algorithms, not isolated
to particular input data.

My gut feeling is: we should "deprecate" these algorithms (which means
they continue to work for existing models, but are hidden from the UI
and cannot be run from the toolbox or added to new models). I don't
see the value in exposing broken algorithms when we have robust
alternatives (the native QGIS difference/symmetric difference
algorithms are very fast, stable, and well tested), and potentially a
lot of harm.

Thoughts?

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

_______________________________________________
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: What to do about incorrect SAGA algorithms?

Nyall Dawson
On Mon, 4 Mar 2019 at 21:59, Pedro Venâncio <[hidden email]> wrote:

>
> Hi Nyall,
>
> I think this rises another problem, that is the use of a very old version of SAGA (2.3.2) in QGIS.
>
> I've just tested Difference algorithm with Lene dataset in native SAGA 2.3.2 and 7.2.0, and 2.3.2 gives the problem described, but 7.2.0 gives the correct result:
>
> https://cld.pt/dl/download/2dfb0db5-eb74-4396-ad18-7451fd1fe788/test_saga_232.jpg
>
> https://cld.pt/dl/download/92182907-d37a-4441-891c-3c85064b3100/test_saga_720.jpg

This is really frustrating -- I wonder if someone with links to the
SAGA project could approach them again and gently ask them to consider
a new LTR release? Unfortunately until they do make a new LTR, users
on our Windows installation and through the larger linux distros are
stuck with the 2.3 release. I don't believe either the windows nor
debian/ubuntu/fedora packaging team have any intention of moving to
non-ltr releases.

>
> So, we are faced again with the question of keep an old version of SAGA in QGIS core, or make it as an external plugin, with someone keeping an eye in the changes introduced by SAGA team between versions (as we saw in the past).
>
> Alexander Bruy had made an effort to keep a parallel SAGA plugin that support newer SAGA versions: https://plugins.bruy.me/processing-saga.html
> But it is not in the official repository. Maybe this is the time to discuss again the use of SAGA as an external plugin?

So -- my new proposal would be:

1. Leave the inbuilt support at LTR only. Deprecate all known broken
algorithms to avoid user error and frustration.
2. Copy the saga provider to a NEW "saga-next gen" plugin based
provider, which targets the current SAGA v7 .2 release ONLY. This
would be a community-maintained plugin (although, as a once off
goodwill gesture I'm willing to do the initial plugin setup and host
it on a North Road github repo). This plugin could be installed
alongside the inbuilt SAGA LTR provider without issue, but it would
NOT be included in a default QGIS install and users would have to
manually install it via the plugin installer.

I think this approach is the best solution when could get given the
constraints we are working around.

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: What to do about incorrect SAGA algorithms?

Nyall Dawson
In reply to this post by Giovanni Manghi
On Mon, 4 Mar 2019 at 21:36, Giovanni Manghi <[hidden email]> wrote:

>
> Hi Nyall,
>
>
>>
>> Just raising the discussion about what we should do with SAGA
>> Processing algorithms when the results generated by SAGA are known to
>> be incorrect (i.e. there's a bug in SAGA itself).
>
>
> add some kind of warning (for the tools we know can have issue), do not remove, please. After all we never removed (nor deprecated) the native QGIS tools when we knew (sometimes since long)  that they were spitting very wrong results...

See above for my new proposal. But I'm not convinced by this argument
at all -- it's basically saying "let's accept bugs (and the user data
corruption and frustration which result from them) because we once had
other bugs so it's fair". I understand the background you're coming
from, but we should all move on from the bugginess of 2.18 and just be
happy that 3.x is a MUCH better processing experience all round, and
continue to make it even better by fixing/hiding/removing tools which
give incorrect results.

Nyall



>
> just my 2 cents
>
> -- Giovanni --
>
>
> _______________________________________________
> 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: What to do about incorrect SAGA algorithms?

Nyall Dawson
In reply to this post by Nyall Dawson
On Tue, 5 Mar 2019 at 09:36, Nyall Dawson <[hidden email]> wrote:
>
> 2. Copy the saga provider to a NEW "saga-next gen" plugin based
> provider, which targets the current SAGA v7 .2 release ONLY. This
> would be a community-maintained plugin (although, as a once off
> goodwill gesture I'm willing to do the initial plugin setup and host
> it on a North Road github repo). This plugin could be installed
> alongside the inbuilt SAGA LTR provider without issue, but it would
> NOT be included in a default QGIS install and users would have to
> manually install it via the plugin installer.

Here's an (experimental) working version:
https://plugins.qgis.org/plugins/processing_saga_nextgen/

Seems to be working ok for the couple of algorithms I've run it with,
but definitely experimental only for now ;)

Nyall


>
> I think this approach is the best solution when could get given the
> constraints we are working around.
>
> 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: What to do about incorrect SAGA algorithms?

Nyall Dawson
On Tue, 5 Mar 2019 at 11:45, Nyall Dawson <[hidden email]> wrote:

>
> On Tue, 5 Mar 2019 at 09:36, Nyall Dawson <[hidden email]> wrote:
> >
> > 2. Copy the saga provider to a NEW "saga-next gen" plugin based
> > provider, which targets the current SAGA v7 .2 release ONLY. This
> > would be a community-maintained plugin (although, as a once off
> > goodwill gesture I'm willing to do the initial plugin setup and host
> > it on a North Road github repo). This plugin could be installed
> > alongside the inbuilt SAGA LTR provider without issue, but it would
> > NOT be included in a default QGIS install and users would have to
> > manually install it via the plugin installer.
>
> Here's an (experimental) working version:
> https://plugins.qgis.org/plugins/processing_saga_nextgen/
>
> Seems to be working ok for the couple of algorithms I've run it with,
> but definitely experimental only for now ;)

And here's a PR hiding algorithms with known issues from the toolbox by default:
https://github.com/qgis/QGIS/pull/9378

From the PR:
"By default, hide algorithms with known issues from toolbox and add a
Processing setting to allow these to be shown.
When shown, they are highlighted in red with a tooltip explaining that
the algorithm has known issues.
To be used whenever an algorithm has serious known issues which make
it dangerous to use, and where
we CANNOT fix the underlying issues (e.g. due to issues in 3rd party
dependencies)"

Nyall


>
> Nyall
>
>
> >
> > I think this approach is the best solution when could get given the
> > constraints we are working around.
> >
> > 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: What to do about incorrect SAGA algorithms?

pcav
In reply to this post by Nyall Dawson
Hi all,

On 05/03/19 00:36, Nyall Dawson wrote:

> This is really frustrating -- I wonder if someone with links to the
> SAGA project could approach them again and gently ask them to consider
> a new LTR release?

doing it now.
Thanks for raising this.
--
Paolo Cavallini - www.faunalia.eu
QGIS.ORG Chair:
http://planet.qgis.org/planet/user/28/tag/qgis%20board/
_______________________________________________
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: What to do about incorrect SAGA algorithms?

Pedro Venâncio-2
In reply to this post by Nyall Dawson
Hi Nyall,

A seg, 4/03/2019, 23:36, Nyall Dawson <[hidden email]> escreveu:
On Mon, 4 Mar 2019 at 21:59, Pedro Venâncio <[hidden email]> wrote:
>
> Hi Nyall,
>
> I think this rises another problem, that is the use of a very old version of SAGA (2.3.2) in QGIS.
>
> I've just tested Difference algorithm with Lene dataset in native SAGA 2.3.2 and 7.2.0, and 2.3.2 gives the problem described, but 7.2.0 gives the correct result:


This is really frustrating -- I wonder if someone with links to the
SAGA project could approach them again and gently ask them to consider
a new LTR release?

This would be the best approach, but thinking about 2.3.2, in fact, it does not worked as a really LTR, because if I remeber well, it never received any minor fix. That is, 2.3.2 was the LTR version, and never had any fix in this two years.

So, I don't know if we have any benifit of wait for a new LTR, over just choose any newer SAGA version and work with it. 'We' here necessarely includes Linux packagers and OSGeo4W. 

Thoughts?

Best regards,
Pedro





_______________________________________________
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: What to do about incorrect SAGA algorithms?

Giovanni Manghi
In reply to this post by Nyall Dawson
Hi Nyall,


> add some kind of warning (for the tools we know can have issue), do not remove, please. After all we never removed (nor deprecated) the native QGIS tools when we knew (sometimes since long)  that they were spitting very wrong results...

See above for my new proposal. But I'm not convinced by this argument
at all -- it's basically saying "let's accept bugs (and the user data
corruption and frustration which result from them) because we once had
other bugs so it's fair".

ummm... no, I really didn't meant that. What I was thinking was that SAGA served us well (and will still serve) when we could not really rely on some QGIS native tools (that lead to data corruption/wrong results and user frustration). Also thinking about moving on to newer (bug free) releases I didn't liked the idea to see tools disappear from Processing even if we have very good native tools now. Anyway this is now an already obsolete consideration, the solution you already implemented is good to me.
 
I understand the background you're coming
from

I genuinely do not understand what you mean :)

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: What to do about incorrect SAGA algorithms?

Rainer Hurling
In reply to this post by Nyall Dawson
Hi Nyall,

Thanks for this new plugin. It would be nice, if we could use this
plugin for SAGA GIS >= 7.2., because some of us use SAGA GIS 7.3.0
(devel version).

Many thanks in advance and best wishes,
Rainer Hurling


Am 05.03.19 um 02:45 schrieb Nyall Dawson:

> On Tue, 5 Mar 2019 at 09:36, Nyall Dawson <[hidden email]> wrote:
>>
>> 2. Copy the saga provider to a NEW "saga-next gen" plugin based
>> provider, which targets the current SAGA v7 .2 release ONLY. This
>> would be a community-maintained plugin (although, as a once off
>> goodwill gesture I'm willing to do the initial plugin setup and host
>> it on a North Road github repo). This plugin could be installed
>> alongside the inbuilt SAGA LTR provider without issue, but it would
>> NOT be included in a default QGIS install and users would have to
>> manually install it via the plugin installer.
>
> Here's an (experimental) working version:
> https://plugins.qgis.org/plugins/processing_saga_nextgen/
>
> Seems to be working ok for the couple of algorithms I've run it with,
> but definitely experimental only for now ;)
>
> Nyall
>
>
>>
>> I think this approach is the best solution when could get given the
>> constraints we are working around.
>>
>> 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: What to do about incorrect SAGA algorithms?

Nyall Dawson
On Fri, 8 Mar 2019 at 07:21, Rainer Hurling <[hidden email]> wrote:
>
> Hi Nyall,
>
> Thanks for this new plugin. It would be nice, if we could use this
> plugin for SAGA GIS >= 7.2., because some of us use SAGA GIS 7.3.0
> (devel version).
>

Good point! Can you file a bug ticket on the plugin page and I'll look
into this. Should be a matter of just removing the == part of the
check and replace with a >= part.

Nyall

> Many thanks in advance and best wishes,
> Rainer Hurling
>
>
> Am 05.03.19 um 02:45 schrieb Nyall Dawson:
> > On Tue, 5 Mar 2019 at 09:36, Nyall Dawson <[hidden email]> wrote:
> >>
> >> 2. Copy the saga provider to a NEW "saga-next gen" plugin based
> >> provider, which targets the current SAGA v7 .2 release ONLY. This
> >> would be a community-maintained plugin (although, as a once off
> >> goodwill gesture I'm willing to do the initial plugin setup and host
> >> it on a North Road github repo). This plugin could be installed
> >> alongside the inbuilt SAGA LTR provider without issue, but it would
> >> NOT be included in a default QGIS install and users would have to
> >> manually install it via the plugin installer.
> >
> > Here's an (experimental) working version:
> > https://plugins.qgis.org/plugins/processing_saga_nextgen/
> >
> > Seems to be working ok for the couple of algorithms I've run it with,
> > but definitely experimental only for now ;)
> >
> > Nyall
> >
> >
> >>
> >> I think this approach is the best solution when could get given the
> >> constraints we are working around.
> >>
> >> 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: What to do about incorrect SAGA algorithms?

Nyall Dawson
In reply to this post by Giovanni Manghi
On Wed, 6 Mar 2019 at 22:11, Giovanni Manghi <[hidden email]> wrote:

>
> Hi Nyall,
>
>
>> > add some kind of warning (for the tools we know can have issue), do not remove, please. After all we never removed (nor deprecated) the native QGIS tools when we knew (sometimes since long)  that they were spitting very wrong results...
>>
>> See above for my new proposal. But I'm not convinced by this argument
>> at all -- it's basically saying "let's accept bugs (and the user data
>> corruption and frustration which result from them) because we once had
>> other bugs so it's fair".
>
>
> ummm... no, I really didn't meant that. What I was thinking was that SAGA served us well (and will still serve) when we could not really rely on some QGIS native tools (that lead to data corruption/wrong results and user frustration). Also thinking about moving on to newer (bug free) releases I didn't liked the idea to see tools disappear from Processing even if we have very good native tools now. Anyway this is now an already obsolete consideration, the solution you already implemented is good to me.

Great, I think it's a fair compromise.

>
>>
>> I understand the background you're coming
>> from
>
>
> I genuinely do not understand what you mean :)

What I meant was just that I acknowledge that we've had issues in the
past, and understand that we do have work to do to "regain" trust in
these native algorithms.

Nyall


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