Skip to content

Conversation

srowen
Copy link
Member

@srowen srowen commented Apr 16, 2022

What changes were proposed in this pull request?

Change a comment in ALS code to match impl. The comment refers to taking the absolute value of a Normal(0,1) value, but it doesn't.

Why are the changes needed?

The docs and impl are inconsistent. The current behavior actually seems fine, desirable, so, change the comments.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests

@srowen srowen self-assigned this Apr 16, 2022
@github-actions github-actions bot added the ML label Apr 16, 2022
@HyukjinKwon
Copy link
Member

Merged to master, branch-3.3, branch-3.2 and branch-3.1.

HyukjinKwon pushed a commit that referenced this pull request Apr 17, 2022
…n ALS

### What changes were proposed in this pull request?

Change a comment in ALS code to match impl. The comment refers to taking the absolute value of a Normal(0,1) value, but it doesn't.

### Why are the changes needed?

The docs and impl are inconsistent. The current behavior actually seems fine, desirable, so, change the comments.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing tests

Closes #36228 from srowen/SPARK-38816.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit b2b350b)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
HyukjinKwon pushed a commit that referenced this pull request Apr 17, 2022
…n ALS

### What changes were proposed in this pull request?

Change a comment in ALS code to match impl. The comment refers to taking the absolute value of a Normal(0,1) value, but it doesn't.

### Why are the changes needed?

The docs and impl are inconsistent. The current behavior actually seems fine, desirable, so, change the comments.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing tests

Closes #36228 from srowen/SPARK-38816.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit b2b350b)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
HyukjinKwon pushed a commit that referenced this pull request Apr 17, 2022
…n ALS

### What changes were proposed in this pull request?

Change a comment in ALS code to match impl. The comment refers to taking the absolute value of a Normal(0,1) value, but it doesn't.

### Why are the changes needed?

The docs and impl are inconsistent. The current behavior actually seems fine, desirable, so, change the comments.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing tests

Closes #36228 from srowen/SPARK-38816.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit b2b350b)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
@srowen srowen deleted the SPARK-38816 branch April 21, 2022 13:58
kazuyukitanimura pushed a commit to kazuyukitanimura/spark that referenced this pull request Aug 10, 2022
…n ALS

### What changes were proposed in this pull request?

Change a comment in ALS code to match impl. The comment refers to taking the absolute value of a Normal(0,1) value, but it doesn't.

### Why are the changes needed?

The docs and impl are inconsistent. The current behavior actually seems fine, desirable, so, change the comments.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing tests

Closes apache#36228 from srowen/SPARK-38816.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit b2b350b)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants