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

Transform y into 2D tensor when y.ndim == 1 and likelihood == REGRESSION #240

Merged
merged 4 commits into from
Sep 14, 2024

Conversation

wiseodd
Copy link
Collaborator

@wiseodd wiseodd commented Sep 2, 2024

Fixes #235

Rationale on why we don't want to throw an error:

  • PyTorch works on both these 1D-output cases:
    • model(x).squeeze() with y.shape == model(x).shape[0:-1]
    • model(x) with y.shape == model(x).shape
  • So, in the wild, users' dataloaders vary between those to y.shape's. It's too much to ask to require y.shape == model(x).shape since PyTorch doesn't.

@wiseodd wiseodd added the bug Something isn't working label Sep 2, 2024
@wiseodd wiseodd self-assigned this Sep 2, 2024
Copy link
Owner

@aleximmer aleximmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it. However, I would slightly be in favor of either raising an error or at least a warning if out.ndim != y.ndim because that's the same behavior of MSELoss afaik?

Also a test is failing, any idea why?

laplace/baselaplace.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@runame runame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now

@wiseodd wiseodd merged commit 6b0618a into main Sep 14, 2024
4 checks passed
@wiseodd wiseodd deleted the target-dim branch September 14, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing behavior for one-dimensional targets
3 participants