You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "rmuir (via GitHub)" <gi...@apache.org> on 2024/03/27 23:34:51 UTC

[PR] remove unsupported snowball algorithms [lucene]

rmuir opened a new pull request, #13230:
URL: https://github.com/apache/lucene/pull/13230

   Currently we patch the snowball sources and add two stemmers which are not supported by snowball.
   
   They are not tested by their CI build pipeline, etc:
   
   ```
   # Some other stemmers in the snowball project are not included in the standard
   # distribution. To compile a libstemmer with them in, add them to this list,
   # and regenerate the distribution. (You will need a full source checkout for
   # this.) They are included in the snowball website as curiosities, but are not
   # intended for general use, and use of them is is not fully supported.
   ```
   
   Ref: https://github.com/snowballstem/snowball/blob/master/libstemmer/modules.txt#L48-L61
   
   I'd like to remove these from lucene as well, remove `snowball.patch`, and only use what is supported, to make more progress on normalizing our use of snowball as a library. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3889590960

   On main it seems we're using the 'default' Dutch stemmer, which should be KP -
   
   <img width="2154" height="924" alt="image" src="https://github.com/user-attachments/assets/95430c8f-dd5e-4f43-823f-41092f7764c3" />
   
   https://github.com/snowballstem/snowball/blob/master/algorithms/dutch.sbl
   
   On 10x it still seems to be the dutch_porter stemmer. I guess we could consider upgrading those to latest snowball, although it'll break people's indexes/ analysis chains... so maybe not such a great idea.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3889898424

   It's been two years since this patch was applied. Something tells me that even if someone invests the time to add it back to 10x, before downstream systems pick it up (if they do), they'll move on to Lucene 11... Buf if it scratches your itch - please do provide a patch against branch_10x.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "pweerd (via GitHub)" <gi...@apache.org>.
pweerd commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3890226026

   Very true. 
   Shall I create an enhancement request for this? Or is this comment enough in order to get it in Lucene 11?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "pweerd (via GitHub)" <gi...@apache.org>.
pweerd commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3897635225

   PR at https://github.com/apache/lucene/pull/15706
   
   I get the point about major versions. However this change imposes an *avoidable* burden for users. Especially because it looks like everything works OK after the upgrade, but search sucks...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3895536137

   Like Robert said, major Lucene versions may break backward compatibility in certain areas and they may require full reindexing. This is normal. We should add a section about this to MIGRATE.txt though - like this one before:
   
   https://github.com/apache/lucene/blob/main/lucene/MIGRATE.md#snowball-dependency-upgrade
   
   Would you care to provide a patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "pweerd (via GitHub)" <gi...@apache.org>.
pweerd commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3889266399

   I'm a bit late to the discussion I guess.
   I'm curious why dutch kraaij_pohlmann was removed. The docs clearly state that the quality of this stemmer is way better than the quality of the standard dutch one.
   
   I was hoping that language 'dutch' was now pointing to the  kraaij_pohlmann one, but that is not the case.
   
   To me it seems you ditched the wrong one. 
   I'm a bit confused though, since the snowball project has the kraaij_pohlmann as 'dutch' and the porter variant as 'dutch_porter'.
   However, I'm running Elasticsearch 9 with the dutch stemmer, and it is definitely not the 'kraaij_pohlmann' stemmer.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3892440023

   Main uses kp already, I think. This is now the default in snowball.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3898345265

   > Especially because it looks like everything works OK after the upgrade, but search sucks...
   
   Major revision upgrades are... major. The only way to make it backward-compatible would be to use dutch_porter on the main branch as the default, which isn't good either. I don't think there is a good way out of it. 
   
   Thank you for taking time to report the problem though (and for the patch).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "pweerd (via GitHub)" <gi...@apache.org>.
pweerd commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3889640894

   I agree that replacing the stemmer is a bad idea.
   
   But I think we should re-introduce the KP-stemmer! And rename the snowball stemmers because of backward compatibility.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir merged PR #13230:
URL: https://github.com/apache/lucene/pull/13230


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3892997296

   No. this is exactly what major release are for. read ticket description


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] remove unsupported snowball algorithms [lucene]

Posted by "pweerd (via GitHub)" <gi...@apache.org>.
pweerd commented on PR #13230:
URL: https://github.com/apache/lucene/pull/13230#issuecomment-3892931445

   Yes, I can confirm that main is using the KP stemmer as being 'dutch'.
   However, this imposes a problem, like you stated before: it is *not* backward compatible!
   
   Main silently replaced the dutch stemmer into the KP one. This breaks all existing indexes. So a reindex is now mandatory.
   This is not a good choice IMHO.
   
   It would be better to have dutch the dutch-porter stemmer and dutch_kp the dutch stemmer.
   This makes sure that everything is backward compatible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org