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

Fix output of hyperparameter search notebook #321

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

EdAbati
Copy link
Contributor

@EdAbati EdAbati commented Feb 28, 2023

Hi :)

While going through the examples, I have noticed that one of the cells of the text-classification_hyperparameter-search.ipynb notebook raised an Exception.

I just rerun that notebook, and now it looks fine.

@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 Feb 28, 2023

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

@tomaarsen
Copy link
Member

tomaarsen commented Feb 28, 2023

So this is only reran, no other changes? The diff, and even ReviewNB, is a bit of a mess as you can imagine, haha.
It looks good at a glance.

I think I once started to try and fix this notebook, but I got distracted by trying to track down a memory leak that may not even exist. Oops.

@EdAbati
Copy link
Contributor Author

EdAbati commented Feb 28, 2023

Yes, just rerun it in Colab no changes to the code/markdown!

Oh yeah, the diff in ReviewNB looks messy too. Mmm… The diff on VSCode looked good. 😕 I actually made sure to only commit the outputs to keep the diff small (because Colab changed the metadata of each cell a bit too).

Are there any tricks to make ReviewNB show a more helpful diff?

@tomaarsen tomaarsen added the documentation Improvements or additions to documentation label Feb 28, 2023
@tomaarsen
Copy link
Member

I'm guessing it's not possible. It's a bit surprising, really, because it should just be able to recognize which cells correspond between main and this PR. After all, the number of cells didn't change, and only the outputs differ...

That said, this is great! Thank you for taking the time to fix this for us! I have no issues merging this.

@tomaarsen tomaarsen merged commit b887041 into huggingface:main Feb 28, 2023
@EdAbati EdAbati deleted the hyp-search-notebook-outputs branch March 3, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants