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

Resolve memory allocation issue + minor updates #2

Merged
merged 14 commits into from
Feb 9, 2021

Conversation

nrasadi
Copy link
Contributor

@nrasadi nrasadi commented Jan 20, 2021

As depicted in the original notebook, the user requires at least 28GB free memory space, which may not be available in many cases.
This PR resolves this issue.

Moreover, it comes with minor updates including the way of loading and saving files and showing examples.

images can be load and saved in multiple shards
better way of loading and saving files
update method of showing examples
@calebrob6
Copy link
Owner

calebrob6 commented Jan 20, 2021

Hi @nrasadi, thanks for the contribution!

It looks like there is an issue with the labels, the example images at the end of the notebook do not match up with their labels:
image

can you investigate this?

(just a quick guess, x_val = np.load(str(path_imagenet_val_dataset/"x_val_1.npy")) may should be x_val = np.load(str(path_imagenet_val_dataset/"x_val_0.npy"))?)

@calebrob6
Copy link
Owner

Also, this change will break the second notebook -- it expects x_val.npy to exist. Do you mind changing to second notebook to run on these batched samples and verify that you get the same accuracy results?

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 20, 2021

Hi @nrasadi, thanks for the contribution!

It looks like there is an issue with the labels, the example images at the end of the notebook do not match up with their labels:
image

can you investigate this?

(just a quick guess, x_val = np.load(str(path_imagenet_val_dataset/"x_val_1.npy")) may should be x_val = np.load(str(path_imagenet_val_dataset/"x_val_0.npy"))?)

Oh! Sorry. There was a little mistake. I will take care of it.

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 20, 2021

Also, this change will break the second notebook -- it expects x_val.npy to exist. Do you mind changing to second notebook to run on these batched samples and verify that you get the same accuracy results?

Sure. I will fix this.

Modify path to be compatible with the readme guide
…otebook

However the models accuracy are not close to the reported ones
@calebrob6
Copy link
Owner

It looks like the validation accuracies do not match up now -- my original notebook has top-1 of 0.71256 while yours has 0.61778.

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 21, 2021

I updated the second notebook to be compatible with the first one. However, I don't get the same results.

Reported Accuracy (%) Obtained Results
Top-1 71.3 61.8
Top-5 90.0 84

@calebrob6
Copy link
Owner

Right, can you look into this? I think they should match up

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 21, 2021

It looks like the validation accuracies do not match up now -- my original notebook has top-1 of 0.71256 while yours has 0.61778.

Yes. You're right. But I don't know why!

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 21, 2021

Right, can you look into this? I think they should match up

Yes, of course. But I think the results should be the same. Could it be due to the difference between the Keras versions?

@calebrob6
Copy link
Owner

I'd be surprised as this is a large difference. Are images in the correct order with the labels? Can you confirm that the preprocessing is working in the same way?

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 21, 2021

Could you please have a look at this issue?

images are saved in uint8 rather than float32.
@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 22, 2021

I'd be surprised as this is a large difference. Are images in the correct order with the labels? Can you confirm that the preprocessing is working in the same way?

I double checked the notebooks. They don't seem different than the original ones.

@calebrob6
Copy link
Owner

Something has to be different if the results are different

@nrasadi
Copy link
Contributor Author

nrasadi commented Jan 23, 2021

I'm gonna give up. As the last try, can you find the Keras version you were used to get those results?
Just to make sure nothing is changed since then.

@nrasadi nrasadi closed this Feb 4, 2021
@calebrob6
Copy link
Owner

calebrob6 commented Feb 4, 2021

As the last try, can you find the Keras version you were used to get those results?

Good question, I'll try to reproduce my results now.

@calebrob6
Copy link
Owner

@nrasadi, I'm able to reproduce the results as is with tensorflow=2.3.1 and keras=2.4.3

@nrasadi nrasadi reopened this Feb 6, 2021
@nrasadi
Copy link
Contributor Author

nrasadi commented Feb 6, 2021

@nrasadi, I'm able to reproduce the results as is with tensorflow=2.3.1 and keras=2.4.3

Thank you very much.

@nrasadi
Copy link
Contributor Author

nrasadi commented Feb 6, 2021

I was able to reproduce the results for VGG19 model with Tensorflow==2.3.0.
Top-1 = 71.248 %
Top-5 = 89.986 %

@nrasadi
Copy link
Contributor Author

nrasadi commented Feb 6, 2021

I will gradually check the other available models in the next few days if it would help.

@nrasadi
Copy link
Contributor Author

nrasadi commented Feb 6, 2021

Just in case, I've added a tensor-based top_k_accuracy calculation but I mistakenly haven't created a seperate PR for that.

@calebrob6
Copy link
Owner

Thanks, great stuff @nrasadi ! Just a few changes if you don't mind and I'll be happy to merge!

For 1. Preprocess ImageNet validation set.ipynb:

  • Can you remove tensorflow imports (just to clean it up -- tensorflow isn't used here)
  • Can you hardcode the paths to ILSVRC2012_validation_ground_truth.txt, meta.mat, ... to the data/ directory
  • ut.humansize is not defined

For 2. Benchmark Keras pretrained models on ImageNet:

  • The % complete messages are not behaving correctly.

Remove tensorflow imports
Hardcode data paths
Define humansize function inside the notebook
Fix the behavior of percent_completed messages
@nrasadi
Copy link
Contributor Author

nrasadi commented Feb 9, 2021

Just a few changes if you don't mind and I'll be happy to merge!

I fixed the mentioned issues. Please review them.
Again, thanks for your really helpful notebooks and spending your precious time on this PR. 🙏

@calebrob6 calebrob6 merged commit dd922d2 into calebrob6:master Feb 9, 2021
@calebrob6
Copy link
Owner

Awesome, thanks for the contribution! This repo will definitely be more useful to others now.

Best,
Caleb

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.

2 participants