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

Improve log_ploss #3964

Merged
merged 7 commits into from Feb 29, 2024
Merged

Improve log_ploss #3964

merged 7 commits into from Feb 29, 2024

Conversation

turbotimon
Copy link
Contributor

@turbotimon turbotimon commented Sep 26, 2023

@jph00
Copy link
Member

jph00 commented Oct 15, 2023

Many thanks! This is an nbdev project, so the changes need to be made in the source notebooks, and synced from there.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@turbotimon
Copy link
Contributor Author

Many thanks! This is an nbdev project, so the changes need to be made in the source notebooks, and synced from there.

Thanks for pointing that out and I'm sorry I wasn't aware of it (it's my first post on fastai). I have moved it and hope everything is ok now. If not, please let me know

@jph00
Copy link
Member

jph00 commented Oct 16, 2023

No apologies required! Thanks for putting it in the notebook. So now you need to run nbdev_export to sync your notebook with the library.

@turbotimon
Copy link
Contributor Author

Thanks Jeremy (I should have read the CONTRIBUTION.md first)! I now executed nbdev_export (_modidx.py and metrics.py have changed as well as a consequence) and pushed again . What i still don't get, do I have to execute the 13a_learner.ipynb notebooks as well in order to update the ### Plotting tools section or is this done by the ci/cd pipline?

@jph00
Copy link
Member

jph00 commented Oct 17, 2023

Thanks Jeremy (I should have read the CONTRIBUTION.md first)! I now executed nbdev_export (_modidx.py and metrics.py have changed as well as a consequence) and pushed again . What i still don't get, do I have to execute the 13a_learner.ipynb notebooks as well in order to update the ### Plotting tools section or is this done by the ci/cd pipline?

The docs are updated automatically for showing function signatures. The rest of the notebook is simply rendered as-is -- so if you have any new cell outputs, they will appear in the docs too.

@jph00
Copy link
Member

jph00 commented Oct 17, 2023

That's odd - CI is being rather fussy, and complaining that your modidx file has a difference in line endings. Perhaps you modified it manually after syncing?

@turbotimon
Copy link
Contributor Author

That's odd - CI is being rather fussy, and complaining that your modidx file has a difference in line endings. Perhaps you modified it manually after syncing?

I saw that too. However, I didn't touched it and when I git-reset _modidx.py and run nbdev_export, eol is missing again (also on clean upstream/master), can you reproduce that? I'm using Codespace, so it shouldn't be a problem with my machine too

@turbotimon
Copy link
Contributor Author

I merged the current master, but the problem with _modidx.py didn`t change, so I reset it manually. I hope it is ok not to merge

@turbotimon
Copy link
Contributor Author

@jph00 Hi Jeremy, just wondering if this could be merged or if you need something else? Let me know if somethings missing!

@jph00
Copy link
Member

jph00 commented Feb 29, 2024

Thank you -- sorry I missed your update.

@jph00 jph00 merged commit 80d881e into fastai:master Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants