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.

Sign up for GitHub

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

Jump to bottom

[BugFix] Instantiate 2 losses with different keys #1553

Merged
merged 2 commits into from
Sep 20, 2023
Merged

[BugFix] Instantiate 2 losses with different keys #1553

merged 2 commits into from
Sep 20, 2023

Conversation

matteobettini
Copy link
Contributor

Fixes #1545

@matteobettini
amend
614574e 
Signed-off-by: Matteo Bettini <matbet@meta.com>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 20, 2023
@matteobettini
Copy link
Contributor Author

matteobettini commented Sep 20, 2023 •
edited
Loading

cc @Blonck
does this make sense?

vmoens
vmoens approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

LGTM

@vmoens
Copy link
Contributor

vmoens commented Sep 20, 2023

image
For the record: I would tag this FatFix

@vmoens vmoens added the bug Something isn't working label Sep 20, 2023
@vmoens vmoens changed the title [BigFix] Instantiate 2 losses with different keys [BugFix] Instantiate 2 losses with different keys Sep 20, 2023
@matteobettini
Copy link
Contributor Author

ahahaha it is quite fat, lots of content

@vmoens
Copy link
Contributor

vmoens commented Sep 20, 2023

Seems like it's causing a bug with Dreamer but not sure why, can you look into it?

@matteobettini
Copy link
Contributor Author

oh and it is happening only on gpu, let me try

@vmoens
Copy link
Contributor

vmoens commented Sep 20, 2023

bug seems unrelated to this PR, merging

@vmoens vmoens merged commit 3347077 into pytorch:main Sep 20, 2023
42 of 59 checks passed
vmoens pushed a commit to hyerra/rl that referenced this pull request Oct 10, 2023
@matteobettini @vmoens
[BugFix] Instantiate 2 losses with different keys (pytorch#1553)
b63faba 
Signed-off-by: Matteo Bettini <matbet@meta.com>
@matteobettini matteobettini deleted the fix_loss_keys branch December 4, 2023 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vmoens vmoens vmoens approved these changes

Assignees
No one assigned
Labels
bug Something isn't working CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Cannot instantiate 2 losses with different keys
3 participants
@matteobettini @vmoens @facebook-github-bot