Skip to content

fix(sampling): escalate temperature at the exact retry midpoint#274

Open
Osamaali313 wants to merge 1 commit into
google-deepmind:mainfrom
Osamaali313:fix/sampling-temperature-midpoint
Open

fix(sampling): escalate temperature at the exact retry midpoint#274
Osamaali313 wants to merge 1 commit into
google-deepmind:mainfrom
Osamaali313:fix/sampling-temperature-midpoint

Conversation

@Osamaali313

Copy link
Copy Markdown

Summary

dynamically_adjust_temperature (in concordia/utils/sampling.py) returns 0.0 when attempts == max_attempts / 2, which is reachable whenever max_attempts is even.

temperature = 0.0
if attempts > 1 and attempts < (max_attempts / 2.0):
    temperature = 0.5
elif attempts > (max_attempts / 2.0):
    temperature = 0.75
return temperature

Both branches are strictly exclusive (< max/2 and > max/2), so the exact midpoint falls through to the default 0.0. For example with max_attempts=10, attempts=5 returns 0.0 — dropping the choice-sampling temperature back to zero for that retry instead of escalating it, which defeats the retry-diversity intent at exactly the point where it should be ramping up.

Fix

Make the upper branch inclusive so the midpoint escalates:

elif attempts >= (max_attempts / 2.0):
    temperature = 0.75
  • attempts=5, max=10 now returns 0.75 (was 0.0).
  • All other attempt counts are unchanged (verified for every attempts over max_attempts in 2..20).
  • Odd max_attempts (no integer midpoint) are unaffected.

Tests

Adds concordia/utils/sampling_test.py — the first unit tests for this module — covering the temperature schedule (incl. the midpoint regression) and extract_choice_response. All 11 tests pass.

dynamically_adjust_temperature returned 0.0 when attempts == max_attempts / 2
(reachable whenever max_attempts is even). The two branches were strictly
exclusive (attempts < max/2 -> 0.5, attempts > max/2 -> 0.75), so the exact
midpoint fell through to the default 0.0, dropping the choice-sampling
temperature back to zero for that retry instead of escalating it.

Make the upper branch inclusive (>=) so the midpoint escalates to 0.75. All
other attempt counts are unchanged. Adds the first unit tests for
concordia/utils/sampling.py.
@Osamaali313

Copy link
Copy Markdown
Author

Heads-up on the red Test Concordia run: the failure is in the Typecheck (pytype) step, on files this PR does not touch —
concordia/components/game_master/interrupt_resolution_test.py and interrupt_scheduling_test.py
(No attribute 'expiry' / 'description' / 'entity_name' on None). Both files exist on main, so this is a pre-existing typecheck failure rather than something introduced here; pytype runs repo-wide and ninja stops on the first error, so the whole job goes red.

This PR only changes concordia/utils/sampling.py (the one-line midpoint fix) and adds concordia/utils/sampling_test.py. Both typecheck cleanly in the same run, and the new tests pass. I'd be glad to send a separate PR adding the missing None-narrowing in those interrupt_*_test.py files if that would help get the typecheck green again.

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.

1 participant