Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fix the order of error term's operands #13745

Merged
merged 2 commits into from
Jan 16, 2019
Merged

Fix the order of error term's operands #13745

merged 2 commits into from
Jan 16, 2019

Conversation

wlbksy
Copy link
Contributor

@wlbksy wlbksy commented Dec 29, 2018

Error term should be "actual minus predicted".

This PR should be no op, since all related error terms are either squared or transformed into absolute value.

@wlbksy wlbksy requested a review from szha as a code owner December 29, 2018 04:11
Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

Thanks.
LGTM

@sandeep-krishnamurthy sandeep-krishnamurthy added Gluon pr-awaiting-merge Review and CI is complete. Ready to Merge labels Dec 29, 2018
@@ -101,7 +101,7 @@ def hybrid_forward(self, F, x, *args, **kwargs):
class L2Loss(Loss):
r"""Calculates the mean squared error between `pred` and `label`.
Copy link
Contributor

Choose a reason for hiding this comment

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

by the same reasoning, this should be swapped as label is the subject.

Copy link
Contributor

Choose a reason for hiding this comment

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

same elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good finding!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@vishaalkapoor
Copy link
Contributor

LGTM!

Copy link
Contributor

@stu1130 stu1130 left a comment

Choose a reason for hiding this comment

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

LGTM

@szha szha merged commit 5b011b3 into apache:master Jan 16, 2019
stephenrawls pushed a commit to stephenrawls/incubator-mxnet that referenced this pull request Feb 16, 2019
* fix the order of error term's operands

* address comments
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* fix the order of error term's operands

* address comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Gluon pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants