google-research / vision_transformer

Apache License 2.0
10.48k stars 1.3k forks source link

Solution for vit_jax.ipynb error when using the `tensorflow-datasets==4.8.3` library. #266

Closed limesqueezy closed 1 year ago

limesqueezy commented 1 year ago

When tfds.builder("cifar10").info is run (through input_pipeline.get_dataset_info(dataset, 'train')['num_classes']) a dictionary is returned which does has an empty split attribute. I suppose this was not the case with the tensorflow-datasets version 6 months ago neither was it when tensorflow-datasets==4.8.2, as mentioned here. A minor workaround to make go past this trivial error is to change the order of the cells.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

limesqueezy commented 1 year ago

Apologies for the hasty PR, I have included the outputs and have updated the link you asked.

Regarding the tensorboard cell, I think, you're referring to the pre-trained checkpoint colab because vit_jax.ipynb doesn't have one.

I also updated the pre-trained checkpoint link. The branch that it was pointing at, was linen, which returned a Notebook not found error.

Thanks for suggesting ReviewNB I usually use nbdime. I forced-pushed (bad Ari) so that I diff using ReviewNB.

Let me know if you'd need any other changes :)

andsteing commented 1 year ago

Oh, I just noticed this PR didn't go through with our internal systems, let me look into that...

limesqueezy commented 1 year ago

@andsteing out of curiosity, did you find out why it didn't go through? :)

andsteing commented 1 year ago

We have an internal system that integrate PRs from Github with a separate internal code revisioning system, and makes sure they're both up to date. As part of that integration, there are a bunch of checks that run, and one of these checks was failing. I didn't expect a failure with this CL so I didn't immediately notice that the change didn't go through. Once discovered, I could execute a couple of manual commands to fix the check (which triggered erroneously in this case), and the change was then integrated.