Skip to content

feat: Support for Model artifacts#265

Merged
mercuryseries merged 14 commits into
mainfrom
model-artifact
Feb 12, 2025
Merged

feat: Support for Model artifacts#265
mercuryseries merged 14 commits into
mainfrom
model-artifact

Conversation

@mercuryseries

@mercuryseries mercuryseries commented Feb 3, 2025

Copy link
Copy Markdown
Contributor

Changelogs

  • Add a new Model class to represent a model artifact.
  • Add a new load_model utility function to load a model.

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).

TODO

  • Add some tests

@mercuryseries mercuryseries added the feature Annotates any PR that adds new features; Used in the release process label Feb 3, 2025
@mercuryseries mercuryseries self-assigned this Feb 3, 2025

@cwognum cwognum left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @mercuryseries ! 🙏

Comment thread polaris/model/__init__.py
Comment thread polaris/evaluate/_results.py
Comment thread polaris/evaluate/_results.py
Comment thread polaris/evaluate/_results.py Outdated
Comment thread polaris/hub/client.py
@cwognum

cwognum commented Feb 3, 2025

Copy link
Copy Markdown
Collaborator

@mercuryseries For the failing test case, looks like you were just unlucky! We should set the random seed for that test case so that doesn't happen again.

@jstlaurent jstlaurent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Short and sweet. Very nice. 😄

I do agree with Cas' comment, about making this only available on Results V2, for Benchmark V2s.

Comment thread polaris/hub/client.py Outdated
Comment thread polaris/model/__init__.py
Comment thread polaris/hub/client.py
Comment thread polaris/evaluate/_results.py
@mercuryseries mercuryseries force-pushed the model-artifact branch 4 times, most recently from dded7f3 to ec89949 Compare February 7, 2025 03:56

@cwognum cwognum left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Thanks @mercuryseries ! Nice job on splitting it in a V1 and V2.

Comment thread mkdocs.yml
Comment thread polaris/benchmark/_base.py
Co-authored-by: Cas Wognum <cwognum@users.noreply.github.com>

@Andrewq11 Andrewq11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really nice @mercuryseries, thanks for this! 🚀

Left a few minor comments but LGTM overall.

Comment thread polaris/benchmark/_base.py Outdated
Comment thread polaris/evaluate/_metadata.py
Comment thread polaris/hub/client.py
Comment thread polaris/hub/client.py Outdated
Comment thread polaris/hub/client.py Outdated
Comment thread uv.lock Outdated
Co-authored-by: Andrew Quirke <Andrewq11@users.noreply.github.com>
@mercuryseries mercuryseries merged commit 3d30143 into main Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Annotates any PR that adds new features; Used in the release process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants