Daryl Lim Claude Opus 4.8 (1M context) commited on
Commit
40c571e
·
1 Parent(s): fe1383b

fix: apply review cleanups (comment, isnan guard, stronger cap/swap/temp tests)

Browse files

Low-severity polish from the adversarial review of the hardening commits; the
core logic was verified correct, so this is comment/test/robustness tightening.

- _normalize_params: gate math.isnan behind isinstance(value, float) so a huge
out-of-double-range int (direct callers only; Gradio bounds reject it) falls
through to the clamps instead of raising OverflowError. Docstring notes NaN is
live only for temperature (precision=0 rejects NaN for the integer fields).
- Fix the tolerance-gate comment: 0.1*9 is exactly 0.9, not 0.999… — reword to a
value that actually drifts near 1.0.
- Strengthen tests: pin the exact trimmed token count (90 == 720//8) instead of
a `<= constant` tautology; add a non-divisor beam case (1024,7 -> 102) to lock
floor-division; bracket the temperature tolerance with a 1.0+1e-3 sampling
case; add an RTL-source swap orientation (exercises output_rtl's True branch).

71 fast pass; counts unchanged (tests strengthened in place).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Files changed (2) hide show
  1. app.py +9 -3
  2. tests/test_app.py +13 -7
app.py CHANGED
@@ -122,10 +122,15 @@ def _normalize_params(
122
  direct, and the ZeroGPU duration callable — goes through. ``None``/``NaN`` fall back to the
123
  defaults; values are clamped to the ranges the Advanced ``gr.Number`` controls advertise; and
124
  the token×beam product is capped (``_MAX_TOKEN_BEAM_PRODUCT``) so generation stays within the
125
- GPU time it reserved."""
 
 
 
126
 
127
  def _num(value: float | None, default: float) -> float:
128
- return default if value is None or math.isnan(value) else value
 
 
129
 
130
  beams = int(max(1, min(8, _num(num_beams, 1))))
131
  tokens = int(max(1, min(1024, _num(max_new_tokens, 512))))
@@ -171,7 +176,8 @@ def translate(
171
  # public submit path passes gr.Number values uncast, and a cleared field arrives as
172
  # None/NaN, so coerce and clamp before use (the duration callable normalizes identically).
173
  max_new_tokens, num_beams, temperature = _normalize_params(max_new_tokens, num_beams, temperature)
174
- # Compare with a tolerance so float spinner drift (e.g. 0.1*9 = 0.999…) doesn't trip sampling.
 
175
  sampling = abs(temperature - 1.0) > 1e-6
176
 
177
  tokenizer = _load_tokenizer()
 
122
  direct, and the ZeroGPU duration callable — goes through. ``None``/``NaN`` fall back to the
123
  defaults; values are clamped to the ranges the Advanced ``gr.Number`` controls advertise; and
124
  the token×beam product is capped (``_MAX_TOKEN_BEAM_PRODUCT``) so generation stays within the
125
+ GPU time it reserved. ``None`` reaches here for all three params, but ``NaN`` only for
126
+ temperature — ``gr.Number``'s ``precision=0`` rejects ``NaN`` for the integer fields before
127
+ ``translate()`` runs, so their ``NaN`` handling is defensive (covering direct callers and the
128
+ duration callable)."""
129
 
130
  def _num(value: float | None, default: float) -> float:
131
+ # NaN can only be a float; gate the isnan on isinstance so a huge int (which math.isnan
132
+ # would OverflowError converting to a double) falls through to the clamps instead.
133
+ return default if value is None or (isinstance(value, float) and math.isnan(value)) else value
134
 
135
  beams = int(max(1, min(8, _num(num_beams, 1))))
136
  tokens = int(max(1, min(1024, _num(max_new_tokens, 512))))
 
176
  # public submit path passes gr.Number values uncast, and a cleared field arrives as
177
  # None/NaN, so coerce and clamp before use (the duration callable normalizes identically).
178
  max_new_tokens, num_beams, temperature = _normalize_params(max_new_tokens, num_beams, temperature)
179
+ # Compare with a tolerance so float spinner drift (repeated 0.1 steps land on values like
180
+ # 0.9999999999999999, not a clean 1.0) doesn't trip sampling.
181
  sampling = abs(temperature - 1.0) > 1e-6
182
 
183
  tokenizer = _load_tokenizer()
tests/test_app.py CHANGED
@@ -202,19 +202,19 @@ def test_translate_forwards_generation_params():
202
  def test_translate_applies_token_beam_cap():
203
  """A high token×beam request must reach model.generate with the capped token count (not the
204
  raw value) so generation stays within its GPU reservation."""
205
- import app
206
-
207
  kwargs, _ = _run_translate("Hello", "French (fr)", max_new_tokens=1024, num_beams=8)
208
  assert kwargs["num_beams"] == 8
209
- assert kwargs["max_new_tokens"] * kwargs["num_beams"] <= app._MAX_TOKEN_BEAM_PRODUCT
210
 
211
 
212
  def test_translate_tolerates_near_default_temperature():
213
- """A temperature within ~1e-6 of 1.0 (float spinner drift) stays greedy; a clearly different
214
- value still samples."""
215
  near_one, _ = _run_translate("Hello", "French (fr)", temperature=1.0 - 1e-9)
 
216
  sampled, _ = _run_translate("Hello", "French (fr)", temperature=0.7)
217
  assert "do_sample" not in near_one, "near-1.0 temperature should stay greedy"
 
218
  assert sampled.get("do_sample") is True
219
 
220
 
@@ -238,8 +238,9 @@ def test_normalize_params_caps_token_beam_product():
238
  reservation _estimate_duration grants (the worst case must not outlive its 120s budget)."""
239
  import app
240
 
241
- mnt, beams, _ = app._normalize_params(1024, 8, 1.0)
242
- assert beams == 8 and mnt * beams <= app._MAX_TOKEN_BEAM_PRODUCT
 
243
  # the duration estimate, built on the same normalization, never exceeds its 120s cap
244
  assert app._estimate_duration("hi", "French (fr)", 1024, 8, 1.0) <= 120
245
  # comfortably-small products are left untouched
@@ -293,10 +294,15 @@ def test_swap_flips_rtl_to_follow_text():
293
  new_source, new_target, input_update, output_update = app._swap_languages(
294
  "English (en)", "Arabic (ar)", "Hello", "RTL-text"
295
  )
 
 
296
  assert (new_source, new_target) == ("Arabic (ar)", "English (en)")
297
  # input box now holds the Arabic translation -> RTL; output box holds the English source -> LTR
298
  assert input_update == gr.update(value="RTL-text", rtl=True, text_align="right")
299
  assert output_update == gr.update(value="Hello", rtl=False, text_align="left")
 
 
 
300
 
301
 
302
  def test_translate_with_loading_flips_rtl_for_rtl_target():
 
202
  def test_translate_applies_token_beam_cap():
203
  """A high token×beam request must reach model.generate with the capped token count (not the
204
  raw value) so generation stays within its GPU reservation."""
 
 
205
  kwargs, _ = _run_translate("Hello", "French (fr)", max_new_tokens=1024, num_beams=8)
206
  assert kwargs["num_beams"] == 8
207
+ assert kwargs["max_new_tokens"] == 90 # 720 // 8 — exact trim, not just within budget
208
 
209
 
210
  def test_translate_tolerates_near_default_temperature():
211
+ """A temperature within ~1e-6 of 1.0 (float spinner drift) stays greedy; values outside the
212
+ band (either side) still sample, bracketing the tolerance."""
213
  near_one, _ = _run_translate("Hello", "French (fr)", temperature=1.0 - 1e-9)
214
+ just_outside, _ = _run_translate("Hello", "French (fr)", temperature=1.0 + 1e-3)
215
  sampled, _ = _run_translate("Hello", "French (fr)", temperature=0.7)
216
  assert "do_sample" not in near_one, "near-1.0 temperature should stay greedy"
217
+ assert just_outside.get("do_sample") is True, "1e-3 outside the band should sample"
218
  assert sampled.get("do_sample") is True
219
 
220
 
 
238
  reservation _estimate_duration grants (the worst case must not outlive its 120s budget)."""
239
  import app
240
 
241
+ assert app._normalize_params(1024, 8, 1.0) == (90, 8, 1.0) # 720 // 8 — exact trimmed value
242
+ # non-divisor beam width: floor division leaves the product strictly below the budget
243
+ assert app._normalize_params(1024, 7, 1.0) == (102, 7, 1.0) # 720 // 7 = 102; 102*7 = 714 < 720
244
  # the duration estimate, built on the same normalization, never exceeds its 120s cap
245
  assert app._estimate_duration("hi", "French (fr)", 1024, 8, 1.0) <= 120
246
  # comfortably-small products are left untouched
 
294
  new_source, new_target, input_update, output_update = app._swap_languages(
295
  "English (en)", "Arabic (ar)", "Hello", "RTL-text"
296
  )
297
+ # symmetric orientation: RTL source -> the output box (now holding the source) goes RTL
298
+ rtl_src = app._swap_languages("Arabic (ar)", "English (en)", "RTL-src", "english-tgt")
299
  assert (new_source, new_target) == ("Arabic (ar)", "English (en)")
300
  # input box now holds the Arabic translation -> RTL; output box holds the English source -> LTR
301
  assert input_update == gr.update(value="RTL-text", rtl=True, text_align="right")
302
  assert output_update == gr.update(value="Hello", rtl=False, text_align="left")
303
+ # RTL source: input gets the LTR translation (LTR), output gets the RTL source (RTL)
304
+ assert rtl_src[2] == gr.update(value="english-tgt", rtl=False, text_align="left")
305
+ assert rtl_src[3] == gr.update(value="RTL-src", rtl=True, text_align="right")
306
 
307
 
308
  def test_translate_with_loading_flips_rtl_for_rtl_target():