Github user imatiach-msft commented on a diff in the pull request:
https://github.com/apache/spark/pull/17059#discussion_r103554415
--- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala
---
@@ -82,12 +82,20 @@ private[recommendation] trait ALSModelParams extends
Params with HasPredictionCo
* Attempts to safely cast a user/item id to an Int. Throws an exception
if the value is
* out of integer range.
*/
- protected val checkedCast = udf { (n: Double) =>
- if (n > Int.MaxValue || n < Int.MinValue) {
- throw new IllegalArgumentException(s"ALS only supports values in
Integer range for columns " +
- s"${$(userCol)} and ${$(itemCol)}. Value $n was out of Integer
range.")
- } else {
- n.toInt
+ protected val checkedCast = udf { (n: Any) =>
+ n match {
+ case v: Int => v // Avoid unnecessary casting
+ case v: Number =>
+ val intV = v.intValue()
+ // Checks if number within Int range and has no fractional part.
+ if (v.doubleValue == intV) {
--- End diff --
@datumbox I agree that calling toInt in the previous code was not the best
decision either; if we really wanted to support double type correctly we would
have probably done the check with some precision. Maybe if we add to the
documentation that double types for user/item should be avoided I would be ok
with it.
Also, could you add a test case to verify the exception is thrown for
doubles with a fraction, similar to the test case I sent you (with a catch on
the exception type and withClue on the message)? We should clean up the error
message too, since it is thrown not only when the value is not in Integer range
(Int.Max/Min) but also when there is a fractional part.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
