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

De-emphasise SAGA results when searching in toolbox #35734

Merged
merged 1 commit into from
Apr 13, 2020

Conversation

nyalldawson
Copy link
Collaborator

This change "dims" the results from the SAGA provider when a search
is made in the toolbox, to visually push users towards picking alternative
algorithms instead.

The Processing implementation of SAGA algorithms are a constant source
of critical bugs for users, causing incorrect analysis results. There's
zero community interest in actively maintaining this provider, so we
need to take steps to push users to stop picking these algorithms
wherever alternative (QGIS/GRASS/GDAL based) equivalents exist.

And for 4.0, seriously re-consider dropping this provider from the
out of the box install. We are causing more harm then good by offering
it to users.

This change "dims" the results from the SAGA provider when a search
is made in the toolbox, to visually push users towards picking alternative
algorithms instead.

The Processing implementation of SAGA algorithms are a constant source
of critical bugs for users, causing incorrect analysis results. There's
zero community interest in actively maintaining this provider, so we
need to take steps to push users to stop picking these algorithms
wherever alternative (QGIS/GRASS/GDAL based) equivalents exist.

And for 4.0, seriously re-consider dropping this provider from the
out of the box install. We are causing more harm then good by offering
it to users.
@nyalldawson nyalldawson added the Processing Relating to QGIS Processing framework or individual Processing algorithms label Apr 12, 2020
@github-actions github-actions bot added this to the 3.14.0 milestone Apr 12, 2020
@roya0045
Copy link
Contributor

Since SAGA is not that maintained and since you mentioned that there is no interest. Could it be possible to instead have a more up to data saga inclusion but mostly usable through the console or a generic UI+console?

@nirvn
Copy link
Contributor

nirvn commented Apr 13, 2020

I still think we should go one step further and hide any saga algorithm which duplicates a qgis algorithm.

But back to PR, +1.

@alexbruy
Copy link
Contributor

And +1 to remove SAGA completely (maybe with all other providers, except gdal).

@nyalldawson nyalldawson merged commit 0396162 into qgis:master Apr 13, 2020
@nyalldawson nyalldawson deleted the saga_flags branch April 13, 2020 07:12
@gioman
Copy link
Contributor

gioman commented Apr 14, 2020

@nyalldawson @nirvn et al. While I understand that it would be better to move al this Processing providers to plugins (and when this will happen I expect to have full access to the tools provided by the plugin, so nothing must be hidden or not have the same visibility as native tools) I really don't understand the "hate" towards precious sources of tools like SAGA. I know perfectly that documentation is basically missing, but nevertheless toolboxes like SAGA provide tools that is very unlikely will ever be implemented as native QGIS tools (and is not unusual anyway find the references to academic papers that were used as base for the tools development). The fact that some SAGA tool is buggy is not really a justification for this "FUD", after all from time to time aren't we also shipping buggy tools? S**t happen in any family. Regarding SAGA tools that are available as native QGIS tools don't forget that is not only a matter of duplication, but frequently also a matter of different performance, where sometimes it can be QGIS that works better and other times is not QGIS.

Said that I would have preferred a little bit more of discussion/testing around this patch before merging it.

If we decide to move on (+1 from me. Despite being a Linux user I don't think we should keep holding back the SAGA support to ancient versions just because newer ones are not available in official repositories. We should always look at the big picture, Windows and macOS in this case) and make SAGA a plugin, I guess that we would need to be clear to users and point them to such plugin. At this moment what would be? @alexbruy one? Nyall's one? Should we fork one of them and make it a new one?

@nyalldawson
Copy link
Collaborator Author

I really don't understand the "hate" towards precious sources of tools like SAGA

Let me explain. First up, it's an important clarification to make that the hate ISN'T to SAGA itself, it's to the SAGA Processing provider. That's an important distinction.

Let's look at my history with this provider. I was first exposed to the issues in it way back in the first Nodebo hackfest, when I shared a desk with @volaya while he was explaining to @pcav the difficulties he was having with maintaining the saga description files and trying the keep the provider running whenever the SAGA algorithms changed due to new SAGA versions. Even back in this early days of QGIS Processing the provider was a source of frustration to maintainers (@volaya) and users (@pcav).

The problem seemed to be solved when SAGA launched their LTR version, as this allowed us to stabilise on a single version and avoid all the ongoing issues with version compatibility. Unfortunately this effort was doomed, because upstream quickly moved on from the LTR version and it ultimately saw little attention. Bugs in algorithms and saga itself were being fixed in the newer releases, but not backported -- so QGIS users were still experiencing these issues despite them being fixed upstream.

When QGIS 3.0 came around I spent a LOT of time on this provider. Mostly because I'm passionate about QGIS processing and realise that a lot of Processing's strengths comes through diversity of providers and the ability to use 3rd party applications like SAGA. I wanted to offer users a first-rate experience with all of the out-of-the-box providers. So I spent time cleaning up the provider and adding a ton of unit tests to it, and tried to clear out the bug queue of all outstanding SAGA issues. But again, this effort was ultimately doomed because I couldn't even RUN saga commands on my machine. SAGA LTR would segfault on launch when built with newer gcc and proj dependancies. It was fixed upstream, but not in the LTR. So I couldn't even test the algorithms myself without using a crufty VM running an old Ubuntu release in which the LTR still ran without segfaulting. This burnt me out totally -- it was a stupid process to have to go through, and demonstrated to me that relying on SAGA LTR was a total dead end.

So next attempt I made was to fork the provider and start the "SAGA NG" plugin, with the hope that moving things to a plugin (And allowing it to run on a more modern SAGA release) would start to build community involvement in maintaining the provider. And guess what the result was? NADA. Not a single new maintainer was interested in helping with the provider. @alexbruy found the same thing with his alternative SAGA plugin. Bottom line: the wider community just doesn't care and isn't interested in maintaining this provider.

So now we're left with:

  • no active QGIS maintainers want to touch this provider, we're all burnt out by it
  • no wider community members are interested in adopting it
  • it has serious bugs, the worst of which result in incorrect analysis results.

Now, in my user training and QGIS support I'm seeing a LOT of user frustration coming from this provider. I frequently get issues reported to me regarding QGIS buggy analysis tools, only to trace back the issue and find that the user was using a SAGA based tool when a MUCH better QGIS/GDAL/GRASS alternative exists. E.g. using the SAGA buffer tool. Whenever I ask why they picked these tools I get answers like "oh I just thought that was the right one" or "someone told me to use that one and I don't know why". It's a HUGE source of user frustration and is seriously damaging the reputation of QGIS. And this PISSES me off no end, because we've spent HEAPS of effort on Processing and making sure it's robust and trustworthy.

It's the same feeling I used to get when users would report to me that QGIS was rubbish and crashy, when ultimately the cause behind their issues was the buggy OpenLayers plugin. I HATE this situation -- we DO have a great product, capable of fantastic, reliable analysis, and I want everyone to have the same great experiences I have when using this.

So what should we do? The harsh reality is that the provider has been on life support for years. I strongly believe it's time to pull the plug and just let it die, for the good of the QGIS project itself. It's harming the wider project and is bad for our reputation. It can't be fixed and raised to the same level of quality as the rest of QGIS. We just need to make the hard call, rip it out, offer it as an unsupported, "community maintained" plugin and let it sink or swim based on ultimately how motivated the "wider community" is in maintaining it.

@pcav
Copy link
Member

pcav commented Apr 15, 2020

I must say after a lot of resistance I have to admit @nyalldawson is quite right in his analysis.
There are alternative approaches, but all require mid to long term funding and manpower. Anyone is welcome to help with this.

@gioman
Copy link
Contributor

gioman commented Apr 15, 2020

Let me explain. First up, it's an important clarification to make that the hate ISN'T to SAGA itself, it's to the SAGA Processing provider. That's an important distinction.

Let's look at my history with this provider. I was first exposed to the issues in it way back in the first Nodebo hackfest, when I shared a desk with @volaya while he was explaining to @pcav the difficulties he was having with maintaining the saga description files and trying the keep the provider running whenever the SAGA algorithms changed due to new SAGA versions. Even back in this early days of QGIS Processing the provider was a source of frustration to maintainers (@volaya) and users (@pcav).

The problem seemed to be solved when SAGA launched their LTR version, as this allowed us to stabilise on a single version and avoid all the ongoing issues with version compatibility. Unfortunately this effort was doomed, because upstream quickly moved on from the LTR version and it ultimately saw little attention. Bugs in algorithms and saga itself were being fixed in the newer releases, but not backported -- so QGIS users were still experiencing these issues despite them being fixed upstream.

When QGIS 3.0 came around I spent a LOT of time on this provider. Mostly because I'm passionate about QGIS processing and realise that a lot of Processing's strengths comes through diversity of providers and the ability to use 3rd party applications like SAGA. I wanted to offer users a first-rate experience with all of the out-of-the-box providers. So I spent time cleaning up the provider and adding a ton of unit tests to it, and tried to clear out the bug queue of all outstanding SAGA issues. But again, this effort was ultimately doomed because I couldn't even RUN saga commands on my machine. SAGA LTR would segfault on launch when built with newer gcc and proj dependancies. It was fixed upstream, but not in the LTR. So I couldn't even test the algorithms myself without using a crufty VM running an old Ubuntu release in which the LTR still ran without segfaulting. This burnt me out totally -- it was a stupid process to have to go through, and demonstrated to me that relying on SAGA LTR was a total dead end.

So next attempt I made was to fork the provider and start the "SAGA NG" plugin, with the hope that moving things to a plugin (And allowing it to run on a more modern SAGA release) would start to build community involvement in maintaining the provider. And guess what the result was? NADA. Not a single new maintainer was interested in helping with the provider. @alexbruy found the same thing with his alternative SAGA plugin. Bottom line: the wider community just doesn't care and isn't interested in maintaining this provider.

So now we're left with:

* no active QGIS maintainers want to touch this provider, we're all burnt out by it

* no wider community members are interested in adopting it

* it has serious bugs, the worst of which result in incorrect analysis results.

Now, in my user training and QGIS support I'm seeing a LOT of user frustration coming from this provider. I frequently get issues reported to me regarding QGIS buggy analysis tools, only to trace back the issue and find that the user was using a SAGA based tool when a MUCH better QGIS/GDAL/GRASS alternative exists. E.g. using the SAGA buffer tool. Whenever I ask why they picked these tools I get answers like "oh I just thought that was the right one" or "someone told me to use that one and I don't know why". It's a HUGE source of user frustration and is seriously damaging the reputation of QGIS. And this PISSES me off no end, because we've spent HEAPS of effort on Processing and making sure it's robust and trustworthy.

It's the same feeling I used to get when users would report to me that QGIS was rubbish and crashy, when ultimately the cause behind their issues was the buggy OpenLayers plugin. I HATE this situation -- we DO have a great product, capable of fantastic, reliable analysis, and I want everyone to have the same great experiences I have when using this.

So what should we do? The harsh reality is that the provider has been on life support for years. I strongly believe it's time to pull the plug and just let it die, for the good of the QGIS project itself. It's harming the wider project and is bad for our reputation. It can't be fixed and raised to the same level of quality as the rest of QGIS. We just need to make the hard call, rip it out, offer it as an unsupported, "community maintained" plugin and let it sink or swim based on ultimately how motivated the "wider community" is in maintaining it.

@nyalldawson I totally understand: do not forget the countless time I spent in the past to mantain description files up to date, especially during the period that we supported more than 1 SAGA version and when the SAGA project found "funny" to change names of fundamental parameters in fundamental libraries like io_gdal.

This is the reason why I totally understand that SAGA must go as a core provider and become a plugin.

What I just said was simple:

  1. once is a plugin we should not hide anything or make SAGA tools less important: if a user chooses to install and use SAGA he/she is entitled to the full experience.

  2. we (QGIS) should point the users to the SAGA plugin that will replace the core provier, and my question was simple. What we should use (yours? Alex's? a new one?). We should publish this plugin in the plugins repo as an "official" QGIS plugin?

@gioman
Copy link
Contributor

gioman commented Apr 15, 2020

There are alternative approaches, but all require mid to long term funding and manpower. Anyone is welcome to help with this.

I think is foolish to hope that even in the long term we can get a stack of tools that can replace all the ones that SAGA ships (even more if we look at the more recent versions of SAGA).

A plugin for SAGA supporting a fair recent/stable release it will good enough for the users that anyway will need it. This will make the devs happy (SAGA out of core) and users happy (they will still have SAGA). Of course when this will happen the users will have to be warned that if they want to be able to use SAGA they will have to commit to its maintenance, this is fair enough to me. Also in the plugin we should not keep back the SAGA version supported just because this is not available out of the box on some platforms (yes, I'm speaking about Linux, and I'm a Linux user).

I didn't tried recently @nyalldawson or @alexbruy plugins. Are the authors available to make them (well one of them) the "official" SAGA provider replacement? Just asking to understand how we could process this in the next future, so once is settles we will stop discussing about it and put the responsibility on (interested) users side.

@alexbruy
Copy link
Contributor

once is a plugin we should not hide anything or make SAGA tools less important

I don't think that anyone will hide or mark somehow any tools if they will work reliably. This commit was introduced as NOW provider does not work correctly and has a lot of errors. As it is in core now and we can not fix it we at least can warn users and gently push them in the direction of more reliable tools.

This is just a flag and can be easily removed. Moreover any 3rd party plugin does not have this flag and they will appear in the same way as any other plugin.

we (QGIS) should point the users to the SAGA plugin that will replace the core provier

Personally I will be -1 for this. We should not force users and limit their freedom of choice. We never do this in the past (for example with OpenLayers plugin) and should not do it now.

We should publish this plugin in the plugins repo as an "official" QGIS plugin?

How "official" plugin will be different from the core plugin from user perspective? Wouldn't it mean for users that this plugin is maintained by QGIS developers? Also I don't think that forcing plugin author to publish his code in the specific place is a right thing.

@gioman
Copy link
Contributor

gioman commented Apr 15, 2020

This is just a flag and can be easily removed. Moreover any 3rd party plugin does not have this flag and they will appear in the same way as any other plugin.

ok thanks for the explanation

Personally I will be -1 for this. We should not force users and limit their freedom of choice. We never do this in the past (for example with OpenLayers plugin) and should not do it now.

ok, but at least it would make sense to not make things too confused. If we remove the core provider it should be enough to have out there just 1 replacement plugin. Don't you agree?

How "official" plugin will be different from the core plugin from user perspective? Wouldn't it mean for users that this plugin is maintained by QGIS developers?

I used "official" in quotes just to say that if/when the core provider is removed we should at least point the users to a good alyernative.

Also I don't think that forcing plugin author to publish his code in the specific place is a right thing.

no, I didn't meat that.

@alexbruy
Copy link
Contributor

ok, but at least it would make sense to not make things too confused. If we remove the core provider it should be enough to have out there just 1 replacement plugin. Don't you agree?

Not in the current situation when we have a lot of plugins with duplicated functionality with very little or no difference at all in the official repository and there are no rules about duplicating plugins.

@gioman
Copy link
Contributor

gioman commented Apr 15, 2020

Not in the current situation when we have a lot of plugins with duplicated functionality with very little or no difference at all in the official repository and there are no rules about duplicating plugins.

ok. As far as you know there are more SAGA provider replacement plugins others that yours and Nyall's?

@alexbruy
Copy link
Contributor

As far as you know there are more SAGA provider replacement plugins others that yours and Nyall's?

My own needs already covered, so I don't follow any rumors/news is this area and also don't follow updates in QGIS plugin repository. Maybe there are others.

@gioman
Copy link
Contributor

gioman commented Apr 15, 2020

My own needs already covered, so I don't follow any rumors/news is this area and also don't follow updates in QGIS plugin repository. Maybe there are others.

ok I understand. Well if it is of any help for me you can move on and remove SAGA from core @alexbruy @nyalldawson The last time I checked both yours and Nyall's plugins were working ok, and if it's time to move on I can't see why waiting any further.

@alexbruy
Copy link
Contributor

Not sure if we can remove it right now, this can be considered as API break. Probably there are users with scripts/models which still rely on the core provider. I'm afraid we have lost right moment and now should wait for 4.0.

@pcav
Copy link
Member

pcav commented Apr 15, 2020

@alexbruy just to clarify "there are no rules about duplicating plugins": it is correct that there are no rules, but I try to convince all new plugin authors not to duplicate existing functionalities, to merge their work with that of others, or drop it altogether. This has been rather successful in the last years.

@nyalldawson
Copy link
Collaborator Author

@gioman

I totally understand: do not forget the countless time I spent in the past to mantain description files up to date, especially during the period that we supported more than 1 SAGA version and when the SAGA project found "funny" to change names of fundamental parameters in fundamental libraries like io_gdal.

Sorry, I didn't mean to downplay your contributions. I was just stating "my" story with respect to this provider and why I was annoyed by it!

once is a plugin we should not hide anything or make SAGA tools less important: if a user chooses to install and use SAGA he/she is entitled to the full experience.

I think @alexbruy has already covered this -- it's just a flag we'd remove from the plugin, and I agree that if a user has opted in to install the provider then we don't need to penalise the search results

we (QGIS) should point the users to the SAGA plugin that will replace the core provier, and my question was simple. What we should use (yours? Alex's? a new one?).

Good question. Mine is really just the core provider + a few code cleanups + removing the check for SAGA 2. I kept/keep meaning to port the code cleanups back to here, but never got the motivation to do it. As far as I can remember there hasn't been a single update to the description files to get things working better with SAGA > 2.

@alexbruy's on the other hand is a dramatically different approach, and more future proof in my opinion.

Mine is probably the "safest" choice in that it differs little from the current one, Alex's is the "better" choice! I'd be more than happy to close mine off and mark it deprecated so that there's only a single alternative.

I think is foolish to hope that even in the long term we can get a stack of tools that can replace all the ones that SAGA ships (even more if we look at the more recent versions of SAGA).

My immediate goal would be to ensure we have a direct replacement for all saga vector algs. (If we don't already have this, there wouldn't be much left to do...). Rasters are a different story, and that's where SAGA's strengths lie. I think a lot of the SAGA raster algorithms are very specific and we shouldn't replicate them. But there are some generic raster handling algorithms offered by SAGA which I DO think there's value in recreating using native QGIS API. So my goal re rasters would never be a 1:1 feature match!

@pcav
Copy link
Member

pcav commented Apr 16, 2020

I think vector algs (as well as many generic rasters ones) in SAGA have sometimes useful additional options that are sometimes life savers. So I suggest to be extra careful before dismissing an alg just because we have a nominal equivalent in core.
I agree that recreating very specific algs would be rather foolish, and this is where integration has its ultimate advantages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants