refactor: change metric evaluator name adapt to eval_at #1570
Conversation
Latency summaryCurrent PR yields:
Breakdown
Backed by latency-tracking. Further commits will update this comment. |
Is it exactly the same? if it is exactly the same yes we can remove it, otherwise we rename one. I saw, I want to change to
Would u have time to do these couple of changes on this branch? |
|
sure - I do it |
| ground_truth_pairs): | ||
| ruuningavg_rank_evaluate_driver.attach(executor=PrecisionEvaluator(eval_at=2), pea=None) | ||
| ruuningavg_rank_evaluate_driver._apply_all(ground_truth_pairs) | ||
| for pair in ground_truth_pairs: | ||
| doc = pair.doc | ||
| assert len(doc.evaluations) == 1 | ||
| assert doc.evaluations[0].op_name == 'Precision@N' | ||
| assert doc.evaluations[0].op_name == 'Precision@2' |
There was a problem hiding this comment.
can u change this to have
driver.attach(executor=executor, pea=None)
assert driver.op_name == executor.metric
Naming might be wrong, but like this the responsibility of testing the actual name is tested in the executor while the driver tests that the name is taken from executor
|
@florian-hoenicke if u can, can u try running jina hello-world and see if the evaluation results are properly shown in the html resulting? |
Codecov Report
@@ Coverage Diff @@
## master #1570 +/- ##
==========================================
+ Coverage 83.44% 84.34% +0.89%
==========================================
Files 128 128
Lines 6646 6705 +59
==========================================
+ Hits 5546 5655 +109
+ Misses 1100 1050 -50
Continue to review full report at Codecov.
|
|
As a result I get: |
thats okey I just want to check the name |
it was to mantain alignment with hub but yes we can do this. Just wanted to align all |
|
Will do as @hanxiao suggests |
|
I created a new pr for the two tests having the same method name. |
95c50e2 to
426cba6
Compare
|
The unit test failed |
6db2f84 to
ca7aa40
Compare
Thanks for pointing it out |
No description provided.