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

FIX: bugs in evaluator #590

Merged
merged 27 commits into from
Dec 24, 2020
Merged

FIX: bugs in evaluator #590

merged 27 commits into from
Dec 24, 2020

Conversation

guijiql
Copy link
Contributor

@guijiql guijiql commented Dec 18, 2020

1.Individual Evaluator can't raise NotImplementedError when used with eval_setting is full.
2.metrics may be disordered in log information.
3.GAUC will be calculated by error when there are items with same predicted scores for one user.

@chenyushuo chenyushuo requested a review from tsotfsk December 21, 2020 08:50
@tsotfsk
Copy link
Contributor

tsotfsk commented Dec 21, 2020

@guijiql Add some test cases in the tests/.

for metrics, evaluator in metric_eval_bind:
used_metrics = list(metrics_set.intersection(set(metrics.keys())))
used_metrics = [metric for metric in metrics_list if metric in metrics.keys()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in metrics.keys() -> in metrics

all_with_pos = np.any(pos_len_list == 0)
all_with_neg = np.any(neg_len_list == 0)
non_zero_idx = np.full(len(user_len_list), True, dtype=np.bool)
if all_with_pos:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the meaning of this? why does all_with_pos mean np.any(pos_len_list == 0) , if pos_len_list = array([1,2,3]), then np.any(pos_len_list == 0) is False, so all_with_pos is False?

from recbole.evaluator.metrics import metrics_dict


class TestCases(object):
Copy link
Contributor

@tsotfsk tsotfsk Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, pos_rank_sum can‘t ensure your metric is right, please test the function evaluator.collect to ensure your pos_rank_sum is right. The reason to add the test is that it requires complex logic to calculate it.

@chenyushuo chenyushuo merged commit 94d5bd8 into RUCAIBox:0.2.x Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants