You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by mayya-sharipova <gi...@git.apache.org> on 2017/12/01 01:13:34 UTC
[GitHub] lucene-solr pull request #280: LUCENE-8011: Improve similarity explanations
GitHub user mayya-sharipova opened a pull request:
https://github.com/apache/lucene-solr/pull/280
LUCENE-8011: Improve similarity explanations
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mayya-sharipova/lucene-solr LUCENE-8011-improve-similarity-explanations
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/280.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #280
----
commit c389c4992b66b5ae750ba7aa5b37937ebedc6615
Author: Mayya Sharipova <ma...@elastic.co>
Date: 2017-12-01T01:03:39Z
LUCENE-8011: Improve similarity explanations
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on the issue:
https://github.com/apache/lucene-solr/pull/280
Merged, thank you @mayya-sharipova.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on the issue:
https://github.com/apache/lucene-solr/pull/280
@jpountz Thanks, Adrien, sorry for that. I will correct this, and next time make sure to run the tests before submitting a PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #280: LUCENE-8011: Improve similarity explanations
Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/280#discussion_r155453742
--- Diff: lucene/core/src/java/org/apache/lucene/search/similarities/AfterEffectL.java ---
@@ -34,11 +34,14 @@ public final double score(BasicStats stats, double tfn) {
}
@Override
+ // TODO: add explanation for tfn
+ // Currently not possible, as CheckHits.verifyExplanation fails because
+ // in case of a single sub-expl the test expects
+ // the sub-expl's score to be equal to the parent expl's score
--- End diff --
this should be possible by rebasing or merging master back, I modified CheckHits yesterday so that it allows the score to be different from the parent explanation if the explanation matches `.*, computed as .* from:`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on the issue:
https://github.com/apache/lucene-solr/pull/280
Thanks @mayya-sharipova, this looks great. `ant precommit` complains from some missing docs (the build requires that all public/protected APIs have some minimal documentation), could you fix it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #280: LUCENE-8011: Improve similarity explanations
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/lucene-solr/pull/280
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on the issue:
https://github.com/apache/lucene-solr/pull/280
Thank you Mayya, it's much easier to see where these scores come from now. I tried running tests on your PR, but I'm getting failures. It seems to be due to the fact that some of the explanations that you added look like `computed as x from: ` while the test framework expects `computed as x from:` (no trailing whitespace). Removing these trailing whitespaces in explanations should fix the issue.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on the issue:
https://github.com/apache/lucene-solr/pull/280
Thanks @mayya-sharipova, this looks like great progress to me. Maybe we could go even further and do the following:
- in the Axiomatic similarity, add abstract methods to allow sub classes to explain how tf, ln, etc. are computed,
- make BasicModel.explain abstract to force sub classes to have their own explanation and include the formula,
- make sure that our own sub classes of SimilarityBase extend explain (the one that returns an explanation) and include the formula in the explanation.
For the record, there is not too much concern to have about backward compatibility since most of those classes (eg. Axiomatic, BasicModel) are very expert classes and this changes targets master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on the issue:
https://github.com/apache/lucene-solr/pull/280
No need to be sorry!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on the issue:
https://github.com/apache/lucene-solr/pull/280
@jpountz thank you Adrien, I will work on these classes as well
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #280: LUCENE-8011: Improve similarity explanations
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on the issue:
https://github.com/apache/lucene-solr/pull/280
@jpountz Adrien thanks for your help. Sorry, I will make sure to run `ant precommit` before committing next time. I have pushed another change to address this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org