-
Notifications
You must be signed in to change notification settings - Fork 1
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
WIP MNIST example #20
base: main
Are you sure you want to change the base?
Conversation
Initially, I forgot the
line, which caused it to not train at all. Funnily enough, the model seemed to mostly predict "3" for any input. Now with that line, I get It looks suspiciously like the error path I introduced in the fake voters, which 10% of the time vote for @hannahilea I didn't end up mocking out the model like you suggested yet because I was hitting annoying CUDA errors for some reason. I'll try that next though. |
lolol sure looks like an off-by-one in the classes---your predictions are suspiciously bad-in-a-good-way. Let me take a look at the code. Just to clarify, this is the test set evaluation plot, right? |
Co-authored-by: Hannah Robertson <[email protected]>
Yep! That was after 100 epochs. I made the change you suggested, and also tweaked the voter errors so they make off-by-2 mistakes instead of off-by-1 to see if that would change things here. After 23 epochs, I get so still off-by-1 (and not off-by-2, so it seems separate from the intentional error in the voters). |
Yeah---the fact that you aren't getting algorithm vs expert agreements is pretty suspect too. I think there's still something weird with the labels. i'm trying to repro it locally, but running into |
Huh, nope, my working directory is clean. Maybe I should push a manifest? I'm on 1.5.3 but I'll try 1.4 to see if I can repro |
OH. I definitely know what it is. This is one of those problems that would have been startlingly apparent if the labels on this dataset weren't integers, lolol. As far as I can tell, your model training etc is all fine. The issue shows up during evaluation: basically, all of the evaluation metrics of How could you have known this? Good question, hah. Not sure you could have! We definitely need to add it to the documentation around setting up a classifier and the training harness! We do imply it elsewhere in docs, sort of. E.g., from the definition of
(bolding mine). Then the implicit assumption in the following two definitions
is that each of these hard labels is a class index rather than a class value. In this mnist example, our off by one error is introduced when we define the classes as
Without us realizing it, class "0" now has index 1, class "1" has index 2, etc. (Hooray 1-based indexing! Would our example have succeeded if mnist values were instead 1:10? It sure would have!) Under this index-based label assumption, during evaluation, all labels that are integers that fall outside the range of possible ranges (i.e., To fix our example here, we need to do two things:
(EDIT: I don't actually think we need to change this first point at all, shouldn't matter one bit if the classes are strings or not.
Then an image with label |
Great catch! With those changes, it works-- here is epoch 71 (test evaluation): Thanks for all the refs to the code and such. I'll at least rename the variables in the examples to emphasize that they are class indices, and I'll try to see where else in the lighthouse docs I can add emphasis to that fact. It makes a lot of sense that they are indices, but somehow I didn't think about it at all. Btw, it did run ok for me on Julia 1.4.2 (no scalar indexing error). But I noticed there's two dev'd dependencies, LighthouseFlux itself (dev'd into the docs folder on my clone) and the Lighthouse PR. I wonder if that could be a source of non-reproducibility. Also some previous commits did have a scalar indexing error so maybe that's related. |
pairing with @hannahilea to put up a simple example