Chore: First class predictions #187

kirahowe · 2024-09-01T01:37:54Z

Changelogs

This PR extracts all of the normalization and serialization logic that goes into working with predictions into a standalone class for clarity
Closes Add validators for the structure of predictions supplied to prediction evaluation methods #169
- Predictions are still accepted in a very generous variety of formats and there no breaking changes introduced here, but the structure of provided predictions is now validated and standardizes, with helpful error messages for common error cases.
Closes Standardize the expected type of predictions despite differences in the number of tasks and test sets in a benchmark/competition #178
- Evaluation logic now works with the canonical representation of predictions, for both regular and competition benchmarks.
Competition documentation is removed from the docs while the feature is gated.

Checklist:

Was this PR discussed in an issue? It is recommended to first discuss a new feature into a GitHub issue before opening a PR.
Add tests to cover the fixed bug(s) or the newly introduced feature(s) (if appropriate).
Update the API documentation if a new function is added, or an existing one is deleted.
Write concise and explanatory changelogs above.
If possible, assign one of the following labels to the PR: feature, fix, chore, documentation or test (or ask a maintainer to do it for you).

cwognum

This is already giving me peace of mind! 🧘

Seeing it all clearly in one spot did raise some questions. Let me know if any of it doesn't make sense. Happy to elaborate!

polaris/utils/types.py

tests/test_benchmark_predictions.py

docs/api/competition.evaluation.md

polaris/benchmark/_base.py

polaris/benchmark/predictions/_base.py

polaris/evaluate/utils.py

Nicer union syntax Co-authored-by: Cas Wognum <[email protected]>

Co-authored-by: Cas Wognum <[email protected]>

…tional metadata

Andrewq11

Great work @cwognum and @kiramclean 🚀

I think this PR is very clean and adds much needed structure to prediction objects, thank you. I've added some comments but I don't think any are blocking.

polaris/benchmark/_base.py

polaris/evaluate/_predictions.py

polaris/evaluate/_results.py

polaris/evaluate/utils.py

polaris/utils/types.py

tests/test_benchmark_predictions.py

jstlaurent

This looks very fine indeed. I had some suggestions, nothing blocking.

jstlaurent · 2024-09-26T02:31:17Z

polaris/evaluate/_predictions.py

+        return predictions
+
+    @classmethod
+    def _convert_lists_to_arrays(cls, predictions: IncomingPredictionsType) -> IncomingPredictionsType:


Suggestion
This could go outside the class. Maybe even in the utilsmodule.

jstlaurent · 2024-09-26T02:32:04Z

polaris/evaluate/_predictions.py

+        return True
+
+    @classmethod
+    def _check_test_set_size(


Suggestion
This could also be a function outside the class.

jstlaurent · 2024-09-26T02:32:38Z

polaris/evaluate/_predictions.py

+        predictions = cls._normalize_predictions(predictions, target_labels, test_set_labels)
+
+        # Normalize the predictions to a standard representation
+        cls._check_test_set_size(predictions, test_set_sizes)


Comment
I feel like this would be a better for an after model validator. That would fit better with the intent of the Pydantic validator API.

jstlaurent · 2024-09-26T02:34:06Z

polaris/evaluate/_predictions.py

+            return False
+
+        # Inner-level of the dictionary should correspond to the target columns
+        for _, test_set_predictions in predictions.items():


Suggestion

Suggested change

for _, test_set_predictions in predictions.items():

for test_set_predictions in predictions.values():

jstlaurent · 2024-09-26T02:43:46Z

polaris/evaluate/_predictions.py

+        }
+
+    @classmethod
+    def _normalize_predictions(


Comment
This is very nice. Very soothing to me. 😄

kirahowe added 9 commits August 30, 2024 22:28

add base benchmark predictions class, move tests

6d1ab81

wip, validations working

195b938

trying to make types work with arbitrary incoming values

7ae809e

fix equality checking issue in test. closes #169.

7b4687c

add serializer to predictions

739aea7

remove separate competition predictions

7beaf05

update evaluation usage to work with benchmark predictions instance

64fea34

update test set generation for evaluation

ca37ecc

run ruff autoformatting

eed1c53

kirahowe added the chore label Sep 1, 2024

kirahowe requested a review from cwognum as a code owner September 1, 2024 01:37

cwognum assigned kirahowe Sep 3, 2024

cwognum reviewed Sep 3, 2024

View reviewed changes

kiramclean and others added 11 commits September 4, 2024 13:29

Update polaris/utils/types.py

dbbdb54

Nicer union syntax Co-authored-by: Cas Wognum <[email protected]>

Update polaris/utils/types.py

40f9eb1

Co-authored-by: Cas Wognum <[email protected]>

wip

5eb5d45

Merge branch 'main' into first-class-predictions

07516f3

add small docstring, allow string predictions

5ec8959

safely get predictions in evaluation if available

41e9a80

Merge branch 'main' into first-class-predictions

4084cf3

pass test set names to predictions and check for validity

907b77d

simplify safe_mask

dd78c2d

fix bad docstring path

14311a1

Merge branch 'main' into first-class-predictions

45189c3

cwognum self-assigned this Sep 11, 2024

cwognum changed the title ~~First class predictions~~ Feature: First class predictions Sep 11, 2024

cwognum changed the title ~~Feature: First class predictions~~ Chore: First class predictions Sep 11, 2024

cwognum added 3 commits September 11, 2024 16:59

Reintroduce the CompetitionPredictions class because it includes addi…

e968a9b

…tional metadata

Add back the CompetitonPredictions to the docs

67c8f71

Reordered docs

5170818

cwognum added 5 commits September 12, 2024 11:53

Improved documentation and changed logic to disallow some edge cases

cc2223c

Fixed docs

8a545d7

Remove print statement

175f779

Reorganize code

05af664

Simplified evaluation logic

74f4612

cwognum requested review from jstlaurent, zhu0619 and Andrewq11 September 12, 2024 16:35

Andrewq11 approved these changes Sep 17, 2024

View reviewed changes

cwognum and others added 3 commits September 18, 2024 23:02

Address all PR feedback

76ec79b

Merge branch 'main' into first-class-predictions

87cbfd3

Add extra test case

ddf39a3

jstlaurent approved these changes Sep 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: First class predictions #187

Chore: First class predictions #187

kirahowe commented Sep 1, 2024 •

edited by cwognum

Loading

cwognum left a comment

Andrewq11 left a comment

jstlaurent left a comment

jstlaurent Sep 26, 2024

jstlaurent Sep 26, 2024

jstlaurent Sep 26, 2024

jstlaurent Sep 26, 2024

jstlaurent Sep 26, 2024

	for _, test_set_predictions in predictions.items():
	for test_set_predictions in predictions.values():

Chore: First class predictions #187

Are you sure you want to change the base?

Chore: First class predictions #187

Conversation

kirahowe commented Sep 1, 2024 • edited by cwognum Loading

Changelogs

cwognum left a comment

Choose a reason for hiding this comment

Andrewq11 left a comment

Choose a reason for hiding this comment

jstlaurent left a comment

Choose a reason for hiding this comment

jstlaurent Sep 26, 2024

Choose a reason for hiding this comment

jstlaurent Sep 26, 2024

Choose a reason for hiding this comment

jstlaurent Sep 26, 2024

Choose a reason for hiding this comment

jstlaurent Sep 26, 2024

Choose a reason for hiding this comment

jstlaurent Sep 26, 2024

Choose a reason for hiding this comment

kirahowe commented Sep 1, 2024 •

edited by cwognum

Loading