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

Softmax applied twice when computing loss in LogisticRegression #654

Closed
garryod opened this issue May 26, 2021 · 1 comment · Fixed by #655
Closed

Softmax applied twice when computing loss in LogisticRegression #654

garryod opened this issue May 26, 2021 · 1 comment · Fixed by #655
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@garryod
Copy link
Contributor

garryod commented May 26, 2021

🐛 Bug

In the LogisticRegression model, loss is computed as the negative log-likelihood of the log-softmax of the softmax of the linear classifier outputs. I.e. loss = F.nll_loss(F.log_softmax(F.softmax(self.linear(x))), y)

To Reproduce

Code sample

model = LogisticRegression(42, 42)
batch = (torch.rand(42), torch.rand(42))
model.training_step(batch, 0)

Expected behavior

Network loss should be computed as the cross entropy of predictions and targets (negative log likelihood of the log-softmax of the linear classifier outputs). I.e. loss = F.nll_loss(F.log_softmax(self.linear(x)), y)

Environment

  • PyTorch Version (e.g., 1.0): 1.8.1
  • OS (e.g., Linux): RHEL 8.4
  • How you installed PyTorch (conda, pip, source): pip
  • Build command you used (if compiling from source): N/A
  • Python version: 3.9.2
  • CUDA/cuDNN version: N/A
  • GPU models and configuration: N/A
  • Any other relevant information: None

Additional context

This bug should not cause any serious issue, however it deviates from expected behaviour and reduces value separation which has potential to negatively impact training.
The PyTorch implementation of Cross Entropy Loss includes the log-softmax, as such it expects raw unnormalized scores for each class (docs, functional)
Happy to submit a PR to resolve if deemed appropriate

@garryod garryod added fix fixing issues... help wanted Extra attention is needed labels May 26, 2021
@github-actions
Copy link

Hi! thanks for your contribution!, great first issue!

Borda pushed a commit that referenced this issue Jun 16, 2021
Avoid two softmax calls in LogisticRegression

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@Borda Borda added bug Something isn't working and removed fix fixing issues... labels Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants