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

140/best n sampling #326

Merged
merged 6 commits into from
May 11, 2023
Merged

Conversation

metric-space
Copy link
Contributor

@metric-space metric-space commented Apr 27, 2023

Attempts to deliver work for #140

Fixes #140

Gist

Showcase usage of best-of-n sampling

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 28, 2023

The documentation is not available anymore as the PR was closed or merged.

@metric-space metric-space marked this pull request as ready for review May 2, 2023 03:29
@younesbelkada younesbelkada requested a review from natolambert May 2, 2023 12:21
@@ -0,0 +1,680 @@
{
Copy link
Contributor

@younesbelkada younesbelkada May 2, 2023

Choose a reason for hiding this comment

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

Maybe add a few sentences to explain the motivation of the notebook and what does it showcases


Reply via ReviewNB

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this!
Can you also add few notes in the notebook and the README.md file of examples explaining the demonstration?
From my understanding this is a new rollout strategy that a user can use in replacement of the existing one. So this should be highlighted for better understanding and broader adoption

@metric-space metric-space requested a review from younesbelkada May 3, 2023 02:26
@metric-space
Copy link
Contributor Author

@younesbelkada thank you for the review. I took the liberty of shifting the notebook to a directory within examples and have included a README.md within said directory. Whenver you can, please let me know what you think of it

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

Awesome work! Thanks a lot for your contribution

@younesbelkada younesbelkada requested a review from lvwerra May 3, 2023 08:09
@natolambert
Copy link
Contributor

Sorry was a little swamped with travel and stuff last week, but I'm stoked about this @metric-space !

@metric-space
Copy link
Contributor Author

Hey folks, I just realized that the description was edited to say this fixes #140 (implies finality to me) but I think the notebook is a precursor to actual framework code for best-of-n sampling as put here
#140 (comment)

One idea would be to wrap that logic in a helper class like BestOfNSampler, but starting with a notebook sounds like a good way to go!

Happy to put in the work to get said additions in. Just waiting on more PR reviews to see whether I'm in the clear

@metric-space
Copy link
Contributor Author

Hey folks @natolambert and @younesbelkada, hate to poke/ping people but also don't want this development to remain dormant and go cold. Any word on this?

@lvwerra
Copy link
Member

lvwerra commented May 11, 2023

Looks good to me. Let's merge the notebook and think about BestOfNSampler in a separate PR?

@younesbelkada younesbelkada merged commit ab453ec into huggingface:main May 11, 2023
@metric-space
Copy link
Contributor Author

Thank you folks for this and I appreciate the response. I'll start up a draft PR and conversations on the thread (if needed) around the BestOfNSampler shortly

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.

Add Best of N sampling
5 participants