Skip to content

Conversation

@zhu0619
Copy link
Contributor

@zhu0619 zhu0619 commented Apr 22, 2024

Changelogs


Checklist:

  • 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 or test (or ask a maintainer to do it for you).

discussion related to that PR

@zhu0619 zhu0619 requested a review from cwognum as a code owner April 22, 2024 20:47
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, @zhu0619

For this to be merged, there needs to be a similar PR on the Hub side as well that adds support for these new metrics. Is this something you would feel comfortable doing, @zhu0619 ? If not, I can look into it!

@cwognum
Copy link
Collaborator

cwognum commented Apr 22, 2024

Ah I see you created https://github.com/polaris-hub/polaris-hub/issues/317.

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.

Could we add some test cases? Especially for the multi-class one, this sounds like something that could fail.

@zhu0619
Copy link
Contributor Author

zhu0619 commented Apr 23, 2024

Thanks, @zhu0619

For this to be merged, there needs to be a similar PR on the Hub side as well that adds support for these new metrics. Is this something you would feel comfortable doing, @zhu0619 ? If not, I can look into it!

I will do it.

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.

Hey @zhu0619 , this is probably a bigger change than you anticipated by now! Sorry. Think we're almost there! Let me know if you need help. Happy to jump in.

zhu0619 and others added 6 commits April 25, 2024 16:36
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.

Hey @zhu0619, I had one big question I would be curious to hear your thoughts on. Other than that, just some small comments.

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 @zhu0619 ! This is turning into a nice feature that will make the metric system more flexible moving forward.

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, @zhu0619 ! Looks great!

@zhu0619 zhu0619 merged commit 397a5ea into main Apr 30, 2024
@cwognum cwognum changed the title Metrics absolute average fold error Add 'Absolute Average Fold Error" and "Balanced Average Precision" to the metrics Apr 30, 2024
@cwognum cwognum added the feature Annotates any PR that adds new features; Used in the release process label Apr 30, 2024
@cwognum cwognum deleted the feat/metrics branch April 30, 2024 18:56
@cwognum cwognum changed the title Add 'Absolute Average Fold Error" and "Balanced Average Precision" to the metrics Add "Absolute Average Fold Error" and "Balanced Average Precision" to the metrics Apr 30, 2024
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.

3 participants