Add group-wise inference risks #53
Conversation
|
@MatteoGiomi After rebasing I deleted the branch because of some conflicts, and #52 was automatically closed. |
MatteoGiomi
left a comment
MatteoGiomi
left a comment
There was a problem hiding this comment.
Thanks for this second contribution @itrajanovska, I left a few comments for your consideration.
…ates; Remove old comments; Add RuntimeError test.
tests/test_inference_evaluator.py
Outdated
| with pytest.raises(Exception) as runtime_error: | ||
| evaluate_inference_guesses(guesses=pd.Series(guesses), secrets=secrets, regression=False) | ||
| assert runtime_error.type is RuntimeError |
There was a problem hiding this comment.
pytest lets you check for specific exceptions, so that you don't need a separate assert for the exception type.
| with pytest.raises(Exception) as runtime_error: | |
| evaluate_inference_guesses(guesses=pd.Series(guesses), secrets=secrets, regression=False) | |
| assert runtime_error.type is RuntimeError | |
| with pytest.raises(RuntimeError) as runtime_error: | |
| evaluate_inference_guesses(guesses=pd.Series(guesses), secrets=secrets, regression=False) | |
tests/test_inference_evaluator.py
Outdated
| with pytest.raises(Exception) as runtime_error: | ||
| evaluate_inference_guesses(guesses=pd.Series(guesses), secrets=secrets, regression=False) | ||
| assert runtime_error.type is RuntimeError | ||
| assert "The predictions indices do not match the target indices" in str(runtime_error.value) |
There was a problem hiding this comment.
maybe move this inside the with pytest.raises context.
tests/fixtures.py
Outdated
| samples = pd.read_csv(os.path.join(TEST_DIR_PATH, "datasets", fname), nrows=n_samples) | ||
| return samples.drop_duplicates(subset=deduplicate_on) if deduplicate_on else samples |
There was a problem hiding this comment.
In this case, the number of returned samples is not always n_samples. To fix this one can read the whole dataframe, drop the duplicates, then return the first n_samples rows
tests/test_inference_evaluator.py
Outdated
|
|
||
| def test_evaluator_not_evaluated(): | ||
| df = get_adult("ori", n_samples=10) | ||
| df = get_adult("ori", deduplicate_on=None, n_samples=10) |
There was a problem hiding this comment.
I'd stick to implicitly using the default in case no deduplication is needed.
| df = get_adult("ori", deduplicate_on=None, n_samples=10) | |
| df = get_adult("ori", n_samples=10) |
|
|
||
| group_wise = evaluator.risk_for_groups(confidence_level=0) | ||
|
|
||
| for _, results in group_wise.items(): |
There was a problem hiding this comment.
can you add a test on the value of the risk? that way we are sure that all works e2e?
tests/test_mixed_types_kneigbors.py
Outdated
|
|
||
| def test_mixed_type_kNN(): | ||
| df = get_adult("ori", n_samples=10) | ||
| df = get_adult("ori", deduplicate_on=None, n_samples=10) |
There was a problem hiding this comment.
as mentioned above:
| df = get_adult("ori", deduplicate_on=None, n_samples=10) | |
| df = get_adult("ori", n_samples=10) |
tests/test_mixed_types_kneigbors.py
Outdated
| @pytest.mark.parametrize("n_neighbors, n_queries", [(1, 10), (3, 5)]) | ||
| def test_mixed_type_kNN_shape(n_neighbors, n_queries): | ||
| df = get_adult("ori", n_samples=10) | ||
| df = get_adult("ori", deduplicate_on=None, n_samples=10) |
|
hi @itrajanovska, I gave a final look and left a few minor comments. It's almost ready to go! |
2 participants
We need group wise risks to be able to assess the fairness of the assigned risks within groups
Successor of #48 implementing the computation of the group wise risks (as well as passing a custom ML model).