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

Support only BCEAfterSigmoidLoss and BCEWithLogits but not BCELoss #87

Closed
mali-git opened this issue Sep 2, 2020 · 2 comments · Fixed by #109
Closed

Support only BCEAfterSigmoidLoss and BCEWithLogits but not BCELoss #87

mali-git opened this issue Sep 2, 2020 · 2 comments · Fixed by #109
Assignees

Comments

@mali-git
Copy link
Member

mali-git commented Sep 2, 2020

Currently, we support BCE and BCEAfterSigmoidLoss. However, training will fail when training a model with BCE because we do not provide the option predict_with_sigmoid in the scort_hrt/h/r/t() functions (we only support it in the predict_scores_all_heads/tails/relations() functions) .

We could remove the support for blank BCE, or we need to add the option predict_with_sigmoid in the scoring functions.

@mberr
Copy link
Member

mberr commented Sep 2, 2020

BCE should be BCEWithLogits

You can either use BCEAfterSigmoidLoss or BCEWithLogits.

@mberr mberr changed the title Support only BCEAfterSigmoidLoss Support only BCEAfterSigmoidLoss and BCEWithLogits but not BCELoss Sep 4, 2020
@cthoyt
Copy link
Member

cthoyt commented Oct 16, 2020

Closed b #109

@cthoyt cthoyt closed this as completed Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants