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

Add a warning when some of the modules are in eval mode before the training stage #19820

Closed
mszulc913 opened this issue Apr 26, 2024 · 3 comments · Fixed by #20159
Closed

Add a warning when some of the modules are in eval mode before the training stage #19820

mszulc913 opened this issue Apr 26, 2024 · 3 comments · Fixed by #20159
Labels
discussion In a discussion stage feature Is an improvement or enhancement

Comments

@mszulc913
Copy link

mszulc913 commented Apr 26, 2024

Description & Motivation

I feel like #18951 is not visible enough. The change made it super easy to have silent correctness bugs in the codebase when using libraries like huggingface.

Pitch

Add a warning about modules that are set to eval mode before training.

Alternatives

Update the documentation and tutorials.

Additional context

No response

cc @Borda

@mszulc913 mszulc913 added feature Is an improvement or enhancement needs triage Waiting to be triaged by maintainers labels Apr 26, 2024
@AndreiCComan
Copy link

Hello, thanks @mszulc913 for bringing this up. It seems like when loading a huggingface model using from_pretrained, it defaults its training state to False during the training_step. That's indeed not ideal, and could potentially have some negative consequences, especially regarding components like Dropout in the pretrained model.

@awaelchli
Copy link
Contributor

I prefer not to make a warning. This could confuse users who finetune models where parts of the model are frozen (and thus have to remain in eval mode). This was the main motivation of #18951. The model summary update should give some more visibility into this #19468, it could also be made even more explicit there.

@awaelchli awaelchli added discussion In a discussion stage and removed needs triage Waiting to be triaged by maintainers labels Jun 22, 2024
@mszulc913
Copy link
Author

mszulc913 commented Jul 13, 2024

Thank you @awaelchli for the reply. Indeed, the updated model summary will help a lot. However, I'm not sure if it's enough. My main concern is that pre-trained models are ubiquitous today, and this feature poses a risk of turning users away from PL, because of a regression they don't have time or will to investigate.

The docs should be updated as well, for example here:
https://lightning.ai/docs/pytorch/stable/advanced/transfer_learning.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion In a discussion stage feature Is an improvement or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants