add diversity evaluation metrics#1416
Conversation
YanZhangADS
requested review from
anargyri,
gramhagen,
loomlike,
miguelgfierro,
wutaomsft and
yueguoguo
as code owners
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
gramhagen
left a comment
gramhagen
left a comment
There was a problem hiding this comment.
this looks really great. i haven't had a chance to look closely at the notebook yet though
tests/unit/reco_utils/evaluation/test_spark_evaluation_diversity_metrics.py
Show resolved
Hide resolved
tests/unit/reco_utils/evaluation/test_spark_evaluation_diversity_metrics.py
Outdated
Show resolved
Hide resolved
gramhagen
left a comment
gramhagen
left a comment
There was a problem hiding this comment.
this is looking great! i took a closer look at the notebook and focused most of the suggestions there, but caught a couple small items in the main and test files.
tests/unit/reco_utils/evaluation/test_spark_diversity_evaluator.py
Outdated
Show resolved
Hide resolved
miguelgfierro
left a comment
miguelgfierro
left a comment
There was a problem hiding this comment.
Yan this is really good, I made some suggestions
tests/unit/reco_utils/evaluation/test_spark_diversity_evaluator.py
Outdated
Show resolved
Hide resolved
| def test_catalog_coverage(evaluator, target_metrics): | ||
|
|
||
| c_coverage = evaluator.catalog_coverage() | ||
| assert c_coverage == target_metrics["c_coverage"] | ||
|
|
||
| @pytest.mark.spark | ||
| def test_distributional_coverage(evaluator, target_metrics): | ||
|
|
||
| d_coverage = evaluator.distributional_coverage() | ||
| assert d_coverage == target_metrics["d_coverage"] | ||
|
|
||
| @pytest.mark.spark | ||
| def test_item_novelty(evaluator, target_metrics): | ||
| actual = evaluator.item_novelty().toPandas() | ||
| assert_frame_equal(target_metrics["item_novelty"], actual, check_exact=False, check_less_precise=4) | ||
|
|
||
| @pytest.mark.spark | ||
| def test_user_novelty(evaluator, target_metrics): | ||
| actual = evaluator.user_novelty().toPandas() | ||
| assert_frame_equal(target_metrics["user_novelty"], actual, check_exact=False, check_less_precise=4) | ||
|
|
||
| @pytest.mark.spark | ||
| def test_novelty(evaluator, target_metrics): | ||
| actual = evaluator.novelty().toPandas() | ||
| assert_frame_equal(target_metrics["novelty"], actual, check_exact=False, check_less_precise=4) |
There was a problem hiding this comment.
We also need to check the limits, this is very important to check that the formulas are correct. This would be check perfect novelty and non-novelty, perfect diversity and non-diversity, etc.
There was a problem hiding this comment.
"check the limits" -- what is the precision you would recommend? I am comparing the number to the 4 positions after the decimal point.
There was a problem hiding this comment.
4 position is great, we even have 2 position in some cases
tests/unit/reco_utils/evaluation/test_spark_diversity_evaluator.py
Outdated
Show resolved
Hide resolved
tests/unit/reco_utils/evaluation/test_spark_diversity_evaluator.py
Outdated
Show resolved
Hide resolved
|
I think we should add an entry about the diversity metrics in this notebook: https://github.com/microsoft/recommenders/blob/main/examples/03_evaluate/evaluation.ipynb |
4 participants
Description
Related Issues
Checklist:
staging branchand not tomain branch.