muzairkhattak / ProText

[CVPRW 2024] Official repository of paper titled "Learning to Prompt with Text Only Supervision for Vision-Language Models".
https://muzairkhattak.github.io/ProText/
MIT License
86 stars 4 forks source link

Inconsistency in cfg.TRAINER.PROTEXT.CROSS_DATASET during cross-dataset training #5

Closed yasserben closed 4 months ago

yasserben commented 4 months ago

Hello :smiley:

I am experiencing an issue with the cfg.TRAINER.PROTEXT.CROSS_DATASET variable during cross-dataset training on ImageNet. This setting is supposed to enforce text-only training, using ImageNet class names and excluding image data. However, the code seems to contradict this:

https://github.com/muzairkhattak/ProText/blob/32237a035fbfb71ed733b9c947b973259f923696/Dassl.pytorch/dassl/data/data_manager.py#L28-L30

This suggests that text data loads only when cfg.TRAINER.PROTEXT.CROSS_DATASET is False, which seems incorrect for cross-dataset settings where no images should be loaded. As a result, training crashes at the following line as it tries to process text data that is not present in the batch : https://github.com/muzairkhattak/ProText/blob/32237a035fbfb71ed733b9c947b973259f923696/trainers/protext.py#L293-L294

Could you clarify whether I'm misunderstanding the intended use, or if there might be a bug ?

Additionally, there appears to be a typo in the cross dataset training command in TRAIN.md, where promptsrc should likely be protext

Thank you very much ! :wink:

muzairkhattak commented 4 months ago

Hi @yasserben,

Thank you for the message and highlighting your concerns.

Regarding your questions, kindly note the following:

Could you clarify whether I'm misunderstanding the intended use, or if there might be a bug ?

Yes I checked it and it looks like there was a bug. I have removed the cross-dataset condition in the data_magager.py, and now the model is training as expected. The repository has been now updated.

Additionally, there appears to be a typo in the cross dataset training command in TRAIN.md, where promptsrc should likely be protext

Thanks for pointing this out, I have fixed the typos.

Kindly let me know if you have anything else to discuss.

Kind regards! Muhammad Uzair

yasserben commented 4 months ago

Hi @muzairkhattak

Thank for your answers, everything is clear now :+1:

Best regards, Yasser