fix: add zero-guards for divisor/scale edge cases (#7838) - #8133
fix: add zero-guards for divisor/scale edge cases (#7838)#8133liuyun7345 wants to merge 2 commits into
Conversation
Prevent ZeroDivisionError and silent non-finite propagation at four reported sites: 1. groups._ensure_divisibility: reject denominator == 0 before modulo 2. ThroughputTimer: reject non-positive steps_per_output at init and report-boundary time 3. inference_utils.ceil_div: reject divisor == 0 4. HPU FPQuantizer.dequantize: reject zero/non-finite scales before 1/scale Adds focused unit tests covering each failure mode. Picks up the inactive draft deepspeedai#7855. Fixes deepspeedai#7838 Signed-off-by: liuyun7345 <liuyun7345@sina.com>
| pytest.skip("HPU FPQuantizer builder is not available") | ||
|
|
||
|
|
||
| def test_hpu_fp_quantizer_dequantize_rejects_zero_scale(): |
There was a problem hiding this comment.
These tests seems to be hpu-specific. Can they not be generalized?
There was a problem hiding this comment.
Good point — generalizing is possible, and I'll do it. The 1.0 / scale inversion currently only exists in the HPU FPQuantizer.dequantize, but the same failure mode applies to all backends: quantize computes scale = q_range / max_vals, so inf/nan in the input drives max_vals to inf and silently produces scale = 0.
I'll move the zero/non-finite scale validation up into the shared front-end FP_Quantize.dequantize (deepspeed/ops/fp_quantizer/quantize.py) so it covers CUDA and HPU alike, and relocate the test into tests/unit/ops/fp_quantizer/test_fp_quant.py using the existing accelerator-agnostic pattern (deepspeed.ops.op_builder.FPQuantizerBuilder + module-level skip via __compatible_ops__). The direct op_builder.hpu import goes away.
There was a problem hiding this comment.
Done — pushed as 1098c0a. The zero/non-finite scale validation now lives in the shared front-end FP_Quantize.dequantize / selective_dequantize (deepspeed/ops/fp_quantizer/quantize.py), and the regression tests moved to tests/unit/ops/fp_quantizer/test_fp_quant.py using the accelerator-agnostic pattern (deepspeed.ops.op_builder.FPQuantizerBuilder + module-level skip via __compatible_ops__), exercised through the public FP_Quantize.dequantize API. The HPU-only guard in op_builder/hpu/fp_quantizer.py and the direct op_builder.hpu import in the tests are both gone.
| """ | ||
| Return ceil(a / b). | ||
| """ | ||
| if b == 0: |
There was a problem hiding this comment.
Can you please share the usage case that found this issue?
There was a problem hiding this comment.
Honest answer: there is no production usage case on my side. All four guards in this PR come from issue #7838, which was reported from code inspection with synthetic repros (ceil_div(10, 0) raising a raw ZeroDivisionError), not from a real workload failure.
The case for keeping this guard is narrow: ceil_div is a small shared utility, and the guard turns a cryptic ZeroDivisionError into a clear ValueError at the API boundary, with zero behavior change for valid inputs. That said, if you'd rather not add guards without a demonstrated trigger, I'm happy to drop this hunk.
…ai#8133) Move the zero/non-finite scale validation out of the HPU-only `op_builder/hpu/fp_quantizer.py` and into the shared front-end `FP_Quantize.dequantize` / `selective_dequantize` in `deepspeed/ops/fp_quantizer/quantize.py`. The same `1.0 / scale` failure mode applies to every backend (CUDA included): `quantize` computes `scale = q_range / max_vals`, so inf/nan in the input silently yields `scale = 0` and corrupts dequantized output. One guard at the API boundary covers all backends. Relocate the regression tests from `tests/unit/utils/test_zero_guards.py` into `tests/unit/ops/fp_quantizer/test_fp_quant.py` using the existing accelerator-agnostic pattern (`deepspeed.ops.op_builder.FPQuantizerBuilder` + module-level skip via `__compatible_ops__`), exercising the guard through the public `FP_Quantize.dequantize` API. Drop the direct `op_builder.hpu` import and the now-unused `torch` import from `test_zero_guards.py`. Signed-off-by: liuyun7345 <liuyun7345@sina.com> Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
Fixes #7838 by adding explicit zero/non-finite guards at the four reported sites so DeepSpeed fails early with a clear error instead of raising raw
ZeroDivisionErroror silently propagatinginf/nan.This picks up the inactive draft #7855 (no updates since 2026-02-17).
Changes
deepspeed/utils/groups.py—_ensure_divisibilityassertsdenominator != 0before modulo.deepspeed/utils/timer.py—ThroughputTimerrejects non-positivesteps_per_outputin__init__, and_is_report_boundaryre-checks if the attribute is later mutated.deepspeed/inference/v2/inference_utils.py—ceil_divraisesValueErrorwhenb == 0.op_builder/hpu/fp_quantizer.py—dequantizerejects zero / non-finite scales before computing1.0 / scale.tests/unit/utils/test_zero_guards.py— regression tests for each failure mode (HPU builder tests skip if that module is unavailable).Test plan
_ensure_divisibility(8, 0)raises clear AssertionError_ensure_divisibilityinputs still passceil_div(10, 0)raises ValueErrorceil_divmatchesmath.ceilfor valid inputsThroughputTimer(..., steps_per_output=0/-1)raises ValueErrorsteps_per_output=0is caught in_is_report_boundarydequantizerejects zero / NaN / zero-scalar scales before HPU opFixes #7838