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 ONNX import book section #2059

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

antimora
Copy link
Collaborator

@antimora antimora commented Jul 24, 2024

Pull Request Template

Checklist

  • Made sure the book is up to date with changes in this PR.

Changes

Improved the ONNX import section with more specifics on record types and build parameters.

Testing

Rendered the book (see the rendered page here):

cargo xtask books burn open

@antimora
Copy link
Collaborator Author

CC @SimonBrandner and @skewballfox

@antimora antimora changed the title Improve ONNX importing section Improve ONNX import book section Jul 24, 2024
Copy link

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

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

Looks great!!

Copy link
Member

@laggui laggui left a comment

Choose a reason for hiding this comment

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

I have a couple of comments, but overall the additions to this section are great! Thank you 🙏

burn-book/src/import/onnx-model.md Show resolved Hide resolved
burn-book/src/import/onnx-model.md Show resolved Hide resolved
Comment on lines 62 to 63
4. **Trainability**: Imported models can be further trained or fine-tuned using Burn's native
training loop.
Copy link
Member

Choose a reason for hiding this comment

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

using Burn's native training loop.

Though it was already there before, I find the formulation a bit weird, especially due to that last part. You can write you own training loop if you want 🤷‍♂️

Maybe we can just say: "Imported models can be further trained or fine-tuned with Burn."

burn-book/src/import/onnx-model.md Outdated Show resolved Hide resolved
@antimora antimora requested a review from laggui July 25, 2024 16:47
@antimora
Copy link
Collaborator Author

@laggui
Copy link
Member

laggui commented Jul 30, 2024

@antimora sorry I was out of town, will try to take a look at this tomorrow afternoon!

@antimora
Copy link
Collaborator Author

@antimora sorry I was out of town, will try to take a look at this tomorrow afternoon!

No worries! You will make it up with another upcoming big PR ☺️

Copy link
Contributor

@mepatrick73 mepatrick73 left a comment

Choose a reason for hiding this comment

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

@antimora I think your new fix addresses laggui's concerns. Rest looks good to me!

@antimora antimora merged commit bb13729 into tracel-ai:main Aug 1, 2024
@tiruka
Copy link
Contributor

tiruka commented Aug 2, 2024

@antimora

Hi great committers,

I wonder when and how to deploy the fix of burn-book on the https://burn.dev/book .
As I searched the repo, I could not find any docs and github actions related to publish books, and I have not seen the change of this good pr on https://burn.dev/book/import/onnx-model.html as of now 2024/08/02. Some release cycle?

I believe this content will improve docs so I look forward to being reflected.

Thank you!

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.

5 participants