Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Feb 6, 2015

LogisticRegressionModel's predictPoint should directly use broadcasted weights. This pr also fixes the compilation errors of two unit test suite: JavaLogisticRegressionSuite and JavaLinearRegressionSuite.

Fix compilation error.
@viirya viirya changed the title [SPARK-5652][Mllib] Use broadcasted weights [SPARK-5652][Mllib] Use broadcasted weights in LogisticRegressionModel Feb 6, 2015
@srowen
Copy link
Member

srowen commented Feb 6, 2015

CC @jkbradley for the test compile issue, which looks straightforward, and @dbtsai for the question of weights vs weightMatrix

@SparkQA
Copy link

SparkQA commented Feb 6, 2015

Test build #26914 has finished for PR 4429 at commit 5a797e5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mengxr
Copy link
Contributor

mengxr commented Feb 6, 2015

@viirya Any performance gain by changing weightMatrix to weights? Could you say more about the compilation error? The changes looks good to me, but I didn't see any error before making the change.

@jkbradley
Copy link
Member

The changes LGTM too. That compilation error doesn't happen for me either, but the changes are fine.

The switch to weightMatrix looks correct (so that it uses the broadcast variable).

@mengxr
Copy link
Contributor

mengxr commented Feb 6, 2015

@viirya For the weightMatrix -> weights change. This is inside predictPoint and by definition we should use the weightMatrix from input arguments. Are we serializing weights by putting weights in the closure?

@mengxr
Copy link
Contributor

mengxr commented Feb 6, 2015

Merged into master and branch-1.3. Thanks!

@asfgit asfgit closed this in 80f3bcb Feb 6, 2015
asfgit pushed a commit that referenced this pull request Feb 6, 2015
`LogisticRegressionModel`'s `predictPoint` should directly use broadcasted weights. This pr also fixes the compilation errors of two unit test suite: `JavaLogisticRegressionSuite ` and `JavaLinearRegressionSuite`.

Author: Liang-Chi Hsieh <[email protected]>

Closes #4429 from viirya/use_bcvalue and squashes the following commits:

5a797e5 [Liang-Chi Hsieh] Use broadcasted weights. Fix compilation error.

(cherry picked from commit 80f3bcb)
Signed-off-by: Xiangrui Meng <[email protected]>
@viirya
Copy link
Member Author

viirya commented Feb 7, 2015

weightMatrix is just the broadcasted variable of the local copy of original weights. It is used because we don't want to serialize whole model but just the weights.

For the compilation error, when I run the test, the compiler complains something like Object can't compare to Int.

@viirya viirya deleted the use_bcvalue branch December 27, 2023 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants