8000 Feat/add prediction artifact and upload method by j279li · Pull Request #292 · polaris-hub/polaris · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feat/add prediction artifact and upload method #292

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 36 commits into from
Jun 26, 2025
Merged

Feat/add prediction artifact and upload method #292

merged 36 commits into from
Jun 26, 2025

Conversation

j279li
Copy link
Contributor
@j279li j279li commented Jun 3, 2025

Summary of Changes

This PR introduces a new Prediction artifact, and an associated upload method

New Features

  • Prediction Artifact (Predictions)

    • Added a new Predictions model to represent prediction artifacts as Zarr archives.
    • Includes validators to ensure Zarr structure consistency.
    • Provides convenient properties (columns, dtypes, n_rows, etc.) for quick data exploration.
    • Methods for setting/getting prediction values, creating Zarr archives from dicts, and uploading to the Polaris Hub.

  • Upload API (upload_prediction)

    • Added a new upload_prediction method in PolarisHubClient for uploading prediction artifacts.
    • Handles metadata, manifest, and archive uploads with progress tracking.

@j279li j279li self-assigned this Jun 3, 2025
@j279li j279li added the feature Annotates any PR that adds new features; Used in the release process label Jun 3, 2025
jstlaurent
jstlaurent reviewed Jun 3, 2025
View reviewed changes
Copy link
Contributor
@jstlaurent jstlaurent left a comment

Choose a reason for hiding this comment

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

Left some comments about the model structure. We can chat about it some more if you'd like. 😄

cwognum
cwognum approved these changes Jun 5, 2025
View reviewed changes
8000
Copy link
Collaborator
@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

Nice work, @j279li !

@j279li j279li force-pushed the feat/predictions-submission branch from fe10e34 to db77f00 Compare June 5, 2025 15:23
@j279li
Copy link
Contributor Author
j279li commented Jun 5, 2025

@jstlaurent I've encountered circular import issues with BenchmarkV2Specification, since the benchmark package already imports from the evaluate package. To work around this, I moved the Predictions class into its own package for now. Would that be alright, or do you have other ideas?

@j279li j279li marked this pull request as ready for review June 11, 2025 13:38
danielpeng1
danielpeng1 approved these changes Jun 17, 2025
View reviewed changes
Copy link
Contributor
@danielpeng1 danielpeng1 left a comment

Choose a reason for hiding this comment

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

Nice job!

@cwognum
Copy link
Collaborator
cwognum commented Jun 17, 2025

@j279li Ping me once this is ready for another review! Happy to take a look!

@j279li
Copy link
Contributor Author
j279li commented Jun 17, 2025

@j279li Ping me once this is ready for another review! Happy to take a look!

Hey @cwognum, It's basically good for another review! Predictions upload should be fully functional now.

Andrewq11
Andrewq11 reviewed Jun 17, 2025
View reviewed changes
Copy link
Contributor
@Andrewq11 Andrewq11 left a comment

Choose a reason for hiding this comment

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

Thanks again, @j279li 💪 I've left a few comments we should probably take a look at. It's coming along nicely!

cwognum
cwognum requested changes Jun 18, 2025
View reviewed changes
Copy link
Collaborator
@cwognum cwognum left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @j279li, sorry for not spotting any of this on my first review. I've been out of the loop on recent changes to Polaris, so I may also be missing some context here.

We're making good progress, but I think we should take another look at how we've done things for competitions. Specifically, I would like us to revisit the user-facing API as well as the validation of the Predictions class.

Happy to find some chat to about this tomorrow!

cwognum
cwognum requested changes Jun 23, 2025
View reviewed changes
Copy link
Collaborator
@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

Thanks @j279li , we've made a lot of progress but there are still a couple of thinks I would like us to take a look at. Let me know if you have any questions!

Andrewq11
Andrewq11 reviewed Jun 23, 2025
View reviewed changes
Copy link
Contributor
@Andrewq11 Andrewq11 left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes, @j279li! I just have a few more suggestions and comments.

cwognum
cwognum reviewed Jun 23, 2025
View reviewed changes
Copy link
Collaborator
@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

Sorry for the quick feedback, @j279li , but I saw the new commit and figured I would leave the feedback sooner rather than later.

cwognum
cwognum approved these changes Jun 25, 2025
View reviewed changes
Copy link
Collaborator
@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

I'm left with some minor comments, but this looks good to me! Thanks @j279li, great work ! 🚀

@j279li j279li merged commit 74f7b80 into main Jun 26, 2025
19 checks passed
@cwognum cwognum deleted the feat/predictions-submission branch June 26, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
78D4
Reviewers

@jstlaurent jstlaurent jstlaurent left review comments

@cwognum cwognum cwognum approved these changes

@Andrewq11 Andrewq11 Andrewq11 left review comments

@danielpeng1 danielpeng1 danielpeng1 approved these changes

@mercuryseries mercuryseries Awaiting requested review from mercuryseries

Assignees

@j279li j279li

Labels
feature Annotates any PR that adds new features; Used in the release process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@j279li @cwognum @jstlaurent @danielpeng1 @Andrewq11
0