Closed Anindyadeep closed 5 months ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:30Z ----------------------------------------------------------------
Maybe add some explanation on what is this notebook for and explain the end to end process that is happening in it.
Also instead of # Imports, just write "We will now import necessary modules and functions."
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:31Z ----------------------------------------------------------------
For which task?
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:32Z ----------------------------------------------------------------
maybe mention this is also necessary to pass to the model since model doesn't accept string classes. (you're mentioning briefly below, you can also have this section below instead if you're not using id2label yet).
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:32Z ----------------------------------------------------------------
"Let's see some sample images from the dataset."
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:33Z ----------------------------------------------------------------
I saw here you're doing evaluation split but not doing end of training validation with a separate split, maybe you could have a separate split for this here (if the dataset doesn't come with a final test split)
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:34Z ----------------------------------------------------------------
suggestions for clarity:
"It might be possible that some images in your dataset will be grayscale or transparent (RGBA)."
"We pass the images through the processor to apply the transforms to process and convert them into PyTorch format."
also you could carry label2id here instead
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:36Z ----------------------------------------------------------------
We can set push_to_hub
to True
and later call push_to_hub
on Trainer
after the save so people can learn how to use HF Hub as model registry.
MKhalusova commented on 2023-12-08T17:56:01Z ----------------------------------------------------------------
If we set push_to_hub
to True
, we also need to add a call with notebook_login
:
from huggingface_hub import notebook_login notebook_login()
In this case, the output_dir
will also be the name of the repo where your model checkpoint will be pushed.
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:37Z ----------------------------------------------------------------
We will now save the model.
View / edit / reply to this conversation on ReviewNB
merveenoyan commented on 2023-12-06T16:32:38Z ----------------------------------------------------------------
We can also push this repository to https://huggingface.co/hf-vision organization so people can checkout tensorboard logs and the model card.
I checked the # trainable params with sum([p.numel() for p in model.parameters() if p.requires_grad])
and all the params were trainable. So had to freeze the weights manually.
View entire conversation on ReviewNB
Quite end-to-end and nicely put 💜 left some comments on reviewNB to structure the notebook and also added some comments about using transformers 🤗
Thank you very much for the review @merveenoyan! Will keep all your detailed and helpful suggestions in mind, this was an initial PR to understand more about how everything should be arranged.
I will update this notebook accordingly and also add the mdx file. Thank you! 🤗
If we set push_to_hub
to True
, we also need to add a call with notebook_login
:
from huggingface_hub import notebook_login notebook_login()
In this case, the output_dir
will also be the name of the repo where your model checkpoint will be pushed.
View entire conversation on ReviewNB
Nice Demo @shreydan, as Merve already suggested in some of the comments - feel free to make it a bit more conversational. The focus should be on the code, but there is no harm in writing some more text here and there :slightly_smiling_face:
Keep it up :+1:
@MKhalusova @johko @merveenoyan I've made further updates to my PR as suggested.
Please check the image-classification notebooks:
notebooks/Unit 3 - Vision Transformers/fine-tuning-multilabel-image-classification.ipynb
notebooks/Unit 3 - Vision Transformers/transfer-learning-image-classification.ipynb
For transfer-learning:
For fine-tuning:
Based on further reviews, I was thinking to then create a .mdx
file which contains both transfer learning and fine-tuning parts.
@Anindyadeep can you tell me which files are contributed through this PR since I remember I reviewed some of the notebooks in other PRs that seems like a change in this PR. It would be easier for us to review.
@Anindyadeep can you tell me which files are contributed through this PR since I remember I reviewed some of the notebooks in other PRs that seems like a change in this PR. It would be easier for us to review.
Yeah, so all the sub-topics this includes:
All of these topics are included in the PR as a whole. So, some how I am not sure, but when we merged from @shreydan's main to this repo (after our initial peer reviews), it moved all the new files under this PR.
I got to know the reason, and this could be because the initial PR was set from @shreydan main to this main (for image classification only), and now that we were following the convention of first reviewing from our main and then merge for main review, it automatically collects the file and saves here.
@MKhalusova @merveenoyan @johko
Sorry for this confusion, this PR is for the image classification part under unit-3 vision transformers : fine-tuning and transfer-learning.
We have other PRs for segmentation, object detection, LoRA and Knowledge distillation.
For this PR:
Please check the image-classification notebooks:
- notebooks/Unit 3 - Vision Transformers/fine-tuning-multilabel-image-classification.ipynb
- notebooks/Unit 3 - Vision Transformers/transfer-learning-image-classification.ipynb
For image classification, transfer-learning:
For image classification, fine-tuning:
Based on further reviews, I was thinking to then create a .mdx file which contains both transfer learning and fine-tuning parts.
Hey @shreydan 👋 We currently have transfer learning one and LoRA one already merged. Have you renamed the transfer learning one, it should've been in this PR somewhere. Moreover, can you check if your PR aligns with the merged notebooks? I will check which other PRs you have and merge them and come back to this if it's ok.
@merveenoyan sorry didn't know the transfer learning file was merged, actually I have renamed it to transfer-learning-image-classification.ipynb
since there were object detection and segmentation tasks as well.
@shreydan if you can change the name of the file we can change it in another PR
@shreydan we can first wait for other PRs to be merged and then merge this if it's ok for you
@shreydan if you can change the name of the file we can change it in another PR @merveenoyan what should I rename the file to?
@shreydan we can first wait for other PRs to be merged and then merge this if it's ok for you
okay no issues in that. Sorry for this confusion.
@Anindyadeep @shreydan I think we can merge this one. If you could sync the changes with this repository and solve merge conflicts I'll give a one last review 🤓
Sure we can do that, and then let's get it merged :)
@Anindyadeep do you need help with this?
Hey @merveenoyan, so sorry for the delay, I am bit worked up with some priorities, give me few days I along with @shreydan will solve this very soon :)
Hi @merveenoyan, sorry for the delay, @shreydan has helped us by updating and removing the merge conflicts. Let us know what else is needed for the merge.
hello @merveenoyan sorry for the conflicts, our unit had multiple PRs hence the problems. 1 old notebook was removed and has been updated with 2 new notebooks with regards to ViT image classification (transfer learning & fine tuning). Would really appreciate reviews on these 2 notebooks. Thank you for your patience
@MKhalusova can you skim through the notebooks? it seemed ready to me
Merging :rocket:
@johko @MKhalusova @merveenoyan thank you very much!
This PR is contributed by @shreydan @Anindyadeep @alanahmet @hanouticelina @asusevski @sezan92
In this PR, the following things are added:
The initial code for classification using transfer learning.
google/vit-base-patch16-224
: this is the most downloaded and well-known base transformer model.oxford-iiit pets dataset
: hf-link - quite a popular dataset for image classification on the HF hub.datasets
,evaluate
andtransformers
This initial PR is merged in the forked repo after review by @Anindyadeep @sezan92 and @alanahmet
This also adds the code
Some questions to the reviewers in the main repo.
Or, should we have follow a structure where we have two files for Transfer learning and fine-tuning and inside that, we include all of the tasks at once.
Please note: This PR only contains the Jupyter Notebook, based on the feedback, we will start creating
.mdx
files for the same :)