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

Delete model metadata from schema_*.yaml #2195

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

yifanmai
Copy link
Collaborator

@yifanmai yifanmai commented Jan 4, 2024

Currently, model metadata is duplicated across three files (schema_classic.yaml, schema_lite.yaml, model_metadata.yaml). This makes it difficult to keep model metadata up to date.

Additionally, the frontend currently displays metadata for every model, instead of only models that are available.

This deletes models from schema_classic.yaml and schema_lite.yaml. Model metadata should only be stored in model_metadata.yaml.

Additionally, this changes helm-summarize to only write model metadata from models that were actually used in the suite to the frontend JSON. For instance, this prevents HEIM or VHELM models from showing up on the HELM Lite sub-site.

Also addresses #2197 to make CI pass, by pinning opencv-python==4.9.0.80.

@@ -1321,6 +1369,7 @@ def run_pipeline(self, skip_completed: bool, num_instances: int) -> None:
self.read_overlap_stats()

self.write_executive_summary()
self.write_schema()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason this is moved down?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, write_schema() now needs to go after read_runs(), because the runs are now needed to filter out the models that get written to the schema.

Eventually, it would be good to do a refactor to change all the instance variables to local variables, which will make the inputs and outputs of each step more clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Maybe leave a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, added a comment.

Copy link
Contributor

@percyliang percyliang 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 cleaning this up!

@yifanmai
Copy link
Collaborator Author

yifanmai commented Jan 5, 2024

Drive by addressing #2197 which is breaking CI.

@yifanmai yifanmai merged commit 0c08f9d into main Jan 5, 2024
6 checks passed
@yifanmai yifanmai deleted the yifanmai/fix-delete-models-metadata branch January 5, 2024 01:30
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.

2 participants