szehon-ho commented on code in PR #57810:
URL: https://github.com/apache/spark/pull/57810#discussion_r3726142755
##########
python/pyspark/taskcontext.py:
##########
@@ -278,7 +277,7 @@ def cpus(self) -> int:
def cpuAmount(self) -> float:
"""
- The exact amount of CPUs allocated to the task. This can be fractional
when
+ The amount of CPUs allocated to the task. This can be fractional when
Review Comment:
The PR description's rationale is that converting the exact decimal amount
to a Python `float` can lose precision, and that the docs should state that
constraint. Dropping "exact" makes this not-wrong, but it doesn't tell the
reader anything about the approximation. The JVM side leans the other way --
`TaskContext.cpuAmount()` documents a `BigDecimal`, and `cpus()` points at it
"for the exact value" -- so someone comparing the two would reasonably assume
parity. Maybe say the value is returned as a `float` and can differ slightly
from the exact decimal amount?
##########
core/src/main/scala/org/apache/spark/resource/TaskResourceRequest.scala:
##########
@@ -32,9 +32,9 @@ import org.apache.spark.annotation.{Since, Stable}
* numbers, since a task's amount must map onto discrete
resource addresses -
* ie amount equals 0.5 translates into 2 tasks per resource
address. CPUs
* (resource name "cpus") are a plain quantity drawn from the
executor's core
- * pool rather than an addressable resource, so any amount of at
least 1e-9 is
- * valid, e.g. 1.5; the cpus amount is rounded to the nearest
1e-9, so precision
- * beyond 9 decimal places is not preserved.
+ * pool rather than an addressable resource, so any amount from
1e-9 through
+ * Int.MaxValue is valid, e.g. 1.5; the cpus amount is rounded
to the nearest 1e-9,
+ * so precision beyond 9 decimal places is not preserved.
Review Comment:
Both sibling docs qualify this bound with "after rounding" --
`TaskResourceRequests.cpus` just below, and the PySpark
`TaskResourceRequests.cpus` docstring ("valid from 1e-9 to 2147483647 after
rounding"). The qualifier matters here: the check is
`CpuAmount.isInRange(CpuAmount.normalize(...))`, so the bound applies to the
value after HALF_UP rounding to scale 9, and a `Double` half a step outside the
range is accepted -- `5e-10` is the example the `TaskResourceRequests` scaladoc
itself gives. The next clause does mention the rounding, but a reader taking
"from 1e-9 through Int.MaxValue" at face value gets a slightly narrower range
than what the code enforces. Adding "after rounding" would make all three read
the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
