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

MNIST example from the cargo book not working and/or deviating from the official mnist example from the examples #1976

Open
exo-cortex opened this issue Jul 5, 2024 · 9 comments

Comments

@exo-cortex
Copy link

exo-cortex commented Jul 5, 2024

Hi,
The official mnist-code-along-example from the burn book has numerous problems. If I use the declared version 0.13.2 with the declared features and use only the code form the burn book and copy it there are a few compiler errors:

  • first the Module-derive-macro used in model.rs panics apparently.
proc-macro panicked: Modules should be generic over a backend.
    - The generic argument named `B` should have its first trait bound being a backend trait.
    - The default backend trait is `burn::tensor::backend::Backend`.
  • Then in training.rs in the function train when defining the variable learner I get this problem:
error[E0277]: the trait bound `Model<B>: AutodiffModule<_>` is not satisfied
  --> src/training.rs:47:26
   |
47 |         TrainOutput::new(self, item.loss.backward(), item)
   |         ---------------- ^^^^ the trait `AutodiffModule<_>` is not implemented for `Model<B>`
   |         |
   |         required by a bound introduced by this call
   |
   = help: the following other types implement trait `AutodiffModule<B>`:
             bool
             i8
             i16
             i32
             i64
             usize
             u8
             u16
           and 65 others
note: required by a bound in `TrainOutput::<TO>::new`
  --> /home/lroesel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/burn-train-0.13.2/src/learner/train_val.rs:31:39
   |
31 |     pub fn new<B: AutodiffBackend, M: AutodiffModule<B>>(
   |                                       ^^^^^^^^^^^^^^^^^ required by this bound in `TrainOutput::<TO>::new`

Also: The official MNIST example is not the same as the one in the burn book. There are a lot of things different. It would be really nice, if the first thing that people arrive at when learning about burn - the tutorial - is really working and also not that confusing. And even if something does not work one could easily check the working code to compare with one's own example.
It would be nice, if the all needed includes would also be in the code examples. They could also be hidden like the code blocks here in the nominomicon (nom tutorial) where one can click on the eye-symbol to reveal hidden use-statements.

It's really frustrating especially in this mnist example, because a lot of it only works in the end (when every thing comes together). It's a rather large introduction that takes some time to write down. If in the end it doesn't compile its not really useful as a welcoming tool to the world of burn.

@laggui
Copy link
Member

laggui commented Jul 5, 2024

The issue here is that the book is rendered for the last released version (i.e., 0.13.2) while the examples linked are on the main branch.

We could probably link to the release version of the examples: https://github.com/tracel-ai/burn/tree/release/0.13/examples.
If you take a look at the example from there, it should match and work.

Also, the hidden code blocks have been added in PR #1742, but since no release has been made the changes were not applied to the book online.

/edit: at the same time, we do have a warning at the beginning of the guide!

image

@exo-cortex
Copy link
Author

Thank you for your comment!
But even if I check your link (in order to see the examples in version/0.13) the training.rs file has a function run which does not exist in the burn-book example. In the burn-book-example the method is train. So it still deviates in my understanding.

@laggui
Copy link
Member

laggui commented Jul 5, 2024

Thank you for your comment! But even if I check your link (in order to see the examples in version/0.13) the training.rs file has a function run which does not exist in the burn-book example. In the burn-book-example the method is train. So it still deviates in my understanding.

Are you perhaps looking at the mnist example and not the guide?

@exo-cortex
Copy link
Author

Oh, that could be.
But aren't both examples almost identical?
I will check out everything! Thank you.
(I'd like to mention thought that this is kind of confusing. Why are there 2 examples with mnist?)
Could you maybe start the burn-book-example with some text explaining it a bit further, with links to both examples and an explanation to why there are two next to the exact version/commit at which the example is corresponding to the burn-book code-along?

@laggui
Copy link
Member

laggui commented Jul 5, 2024

The examples are not identical. The guide only uses the wgpu backend while the mnist example is configurable to demonstrate how to use different backends.

I agree though that they are pretty similar. But we just added a list of examples in PR #1966 which should (hopefully) help clear up the confusion.

@nathanielsimard
Copy link
Member

Maybe we should remove the MNIST example and only keep the guide, it's probably less confusing. We may simply convert the current mnist example to a more general image-classification example using cifar instead, or something like that.

@antimora
Copy link
Collaborator

antimora commented Jul 6, 2024

Just keep in mind there is web mnist example that uses the output of mnist training. I optimized the model for web.

@exo-cortex
Copy link
Author

Maybe a good change would be to rename the mnist-example to something like "multiple_backends_mnist". I feel like it ist mostly designed to showcase the ease of switching between different backends. Mnist is only a means to a goal.
"guide" could become "guide_mnist_book" or something.

And maybe just maybe I am simply uniquely stupid and this isn't a problem at all and other people have no problem with this issue ;-)

@laggui
Copy link
Member

laggui commented Jul 8, 2024

I think you bring a valid point 🙂 we should probably review the examples name to make them a bit more clear

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

No branches or pull requests

4 participants