Skip to content

Add metric for judging numeric relative error #459

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 1 commit into from
Jun 28, 2021
Merged

Add metric for judging numeric relative error #459

merged 1 commit into from
Jun 28, 2021

Conversation

r-barnes
Copy link
Contributor

@r-barnes r-barnes commented Jun 20, 2021 •
edited
Loading

Also alphabetizes some import lists

Unfortunately, I was unable to test this locally.

@google-cla google-cla bot added the cla: yes contributor license agreement: yes label Jun 20, 2021
@r-barnes r-barnes mentioned this pull request Jun 20, 2021
Closed
RomanPlusPlus
RomanPlusPlus reviewed Jun 20, 2021
View reviewed changes
bigbench/api/test_task_metrics.py Outdated
targets, responses = zip(*test_data)
scores = metrics.exact_str_match_fn(targets=targets, responses=responses)

expected_scores = {"exact_str_match": 9/18}

This comment was marked as resolved.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks!

RomanPlusPlus
RomanPlusPlus reviewed Jun 21, 2021
View reviewed changes
bigbench/api/test_task_metrics.py Outdated
(0, "i am not a number"), # Bad
]
targets, responses = zip(*test_data)
scores = metrics.exact_str_match_fn(targets=targets, responses=responses)

This comment was marked as resolved.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@r-barnes r-barnes force-pushed the richard/add_numeric_rel_error_metric branch from 568b803 to 21175ac Compare June 22, 2021 06:32
@r-barnes
Copy link
Contributor Author

@RomanPlusPlus : I've fixed the issues you pointed out above and this should be ready for review again.

@r-barnes r-barnes force-pushed the richard/add_numeric_rel_error_metric branch 4 times, most recently from b5ca56a to 4f0a818 Compare June 22, 2021 20:37
@r-barnes
Copy link
Contributor Author

@RomanPlusPlus : All tests pass.

@RomanPlusPlus
Copy link
Contributor

@r-barnes, I'm not an assigned reviewer, sorry for being unclear. Not sure why I'm listed as a reviewer in the Reviewers field. But thank you for the fixes anyway!

@r-barnes r-barnes mentioned this pull request Jun 23, 2021
Merged
@guygurari guygurari requested a review from lewkowycz June 24, 2021 19:34
lewkowycz
lewkowycz suggested changes Jun 24, 2021
View reviewed changes
Copy link
Contributor

@lewkowycz lewkowycz left a comment

Choose a reason for hiding this comment

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

Hello, thanks a lot for adding this! It is great to have submitters which add custom metrics! It looks great and I added some comments for minor modificatons.

@r-barnes
Copy link
Contributor Author

@lewkowycz : Thanks for your review. I've made the changes you requested.

@r-barnes r-barnes force-pushed the richard/add_numeric_rel_error_metric branch from 4f0a818 to ec4e54b Compare June 26, 2021 22:02
@r-barnes r-barnes force-pushed the richard/add_numeric_rel_error_metric branch from ec4e54b to a6bb46d Compare June 28, 2021 17:03
@r-barnes
Copy link
Contributor Author

Fixed the metric's name in GENERATIVE_FN to numeric_match_with_0_1_relative_error with the most recent push.

Waiting for this to merge now.

@r-barnes
Copy link
Contributor Author

@lewkowycz : Can we merge this after the tests pass? The tests for #372 won't pass until this lands and our reviewer is withholding acceptance until the tests pass.

@lewkowycz
Copy link
Contributor

Yes, thanks for adding this again!

@lewkowycz lewkowycz merged commit dc11656 into google:main Jun 28, 2021
@r-barnes r-barnes deleted the richard/add_numeric_rel_error_metric branch June 28, 2021 23:51
Sohl-Dickstein pushed a commit that referenced this pull request Jun 29, 2021
@lewkowycz
Merge pull request #459 from r-barnes/richard/add_numeric_rel_error_m…
0d8936d 
…etric

Add metric for judging numeric relative error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@RomanPlusPlus RomanPlusPlus RomanPlusPlus approved these changes

@lewkowycz lewkowycz lewkowycz approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
cla: yes contributor license agreement: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@r-barnes @RomanPlusPlus @lewkowycz