MAINT GCG: make optimization iteration state explicit and typed - #2467
MAINT GCG: make optimization iteration state explicit and typed#2467fei (feiiiiii5) wants to merge 7 commits into
Conversation
Implements the structural half of microsoft#2416 while keeping public attack behavior and extension protocols unchanged. - add StopReason enum plus typed OptimizationRunState and ProgressiveScheduleState dataclasses capturing suffix, losses, best result, counters, and stop reason - MultiPromptAttack.run now tracks state through the typed object and exposes it as last_run_state; stopping and periodic logging phases are extracted into _all_training_prompts_jailbroken and _log_best_checkpoint with stable contracts - ProgressiveMultiPromptAttack.run tracks admission scheduling through ProgressiveScheduleState (exposed as last_schedule_state) and moves final evaluation into _finalize_progressive_run - GCGMultiPromptAttack extracts the candidate-selection phase into _select_best_candidate; candidate batches intentionally remain step-local to bound VRAM - add deterministic seeded regression tests covering stop reasons, best tracking under annealing rejection, checkpoint restore, argmin decomposition across worker groups, and progressive finalize path
181851a to
b718775
Compare
|
Rebased onto current
|
Roman Lutz (romanlutz)
left a comment
There was a problem hiding this comment.
Thanks for the submission! A few things need adjusting, though.
…ring, candidate loss pairing - Replace RST roles with double-backtick references (check-no-rest-roles) - ProgressiveMultiPromptAttack carries the inner loss on ``ProgressiveScheduleState.loss`` instead of a loose local, so ``last_schedule_state.loss`` reflects the final inner run; added a post-loop assertion guarding silent carry-over regressions - ``last_run_state`` / ``last_schedule_state`` default to ``None`` and are cleared at the start of each run, so failed runs expose no stale data - Annealing rejection no longer pairs the accepted suffix with the rejected candidate's loss: ``OptimizationRunState.candidate_loss`` tracks what was just evaluated while ``loss`` stays paired with ``state.control`` Signed-off-by: fei <204683769+feiiiiii5@users.noreply.github.com>
Implements the structural half of microsoft#2416 while keeping public attack behavior and extension protocols unchanged. - add StopReason enum plus typed OptimizationRunState and ProgressiveScheduleState dataclasses capturing suffix, losses, best result, counters, and stop reason - MultiPromptAttack.run now tracks state through the typed object and exposes it as last_run_state; stopping and periodic logging phases are extracted into _all_training_prompts_jailbroken and _log_best_checkpoint with stable contracts - ProgressiveMultiPromptAttack.run tracks admission scheduling through ProgressiveScheduleState (exposed as last_schedule_state) and moves final evaluation into _finalize_progressive_run - GCGMultiPromptAttack extracts the candidate-selection phase into _select_best_candidate; candidate batches intentionally remain step-local to bound VRAM - add deterministic seeded regression tests covering stop reasons, best tracking under annealing rejection, checkpoint restore, argmin decomposition across worker groups, and progressive finalize path
…ring, candidate loss pairing - Replace RST roles with double-backtick references (check-no-rest-roles) - ProgressiveMultiPromptAttack carries the inner loss on ``ProgressiveScheduleState.loss`` instead of a loose local, so ``last_schedule_state.loss`` reflects the final inner run; added a post-loop assertion guarding silent carry-over regressions - ``last_run_state`` / ``last_schedule_state`` default to ``None`` and are cleared at the start of each run, so failed runs expose no stale data - Annealing rejection no longer pairs the accepted suffix with the rejected candidate's loss: ``OptimizationRunState.candidate_loss`` tracks what was just evaluated while ``loss`` stays paired with ``state.control`` Signed-off-by: fei <204683769+feiiiiii5@users.noreply.github.com>
…admission
The progressive-run guard asserted that a measurable loss existed whenever
any optimization step had completed. When an inner run consumes the entire
remaining step budget while a further goal/worker is still waiting to be
admitted, the admission transition resets schedule.loss to inf right
before the budget-exhausted loop exits — so the assertion crashed a valid
run instead of returning. (Two progressive goals at n_steps=3 with an
inner result of ("ctrl", 0.75, 3) reproduces it.)
The guard now only requires a measurable loss while budget remains; a
budget-exhausted exit legitimately carries an inf loss for the goal that
was admitted but never scored.
Regression test reproduces the exact review scenario: goals_admitted=2,
steps_completed=3, loss=inf — the run returns instead of crashing.
cbd449b to
0a68d59
Compare
|
Addressed — thank you for the precise repro. The root cause is exactly as you described: when an inner run consumes the entire remaining step budget while a further goal/worker is still waiting to be admitted, the admission transition resets Fix: the guard now only requires a measurable loss while budget remains ( Regression test reproduces the exact scenario (two progressive goals, |
…ate transitions on budget Review round two on microsoft#2416: - Seed run-state current/best loss from prev_loss instead of a 1e6 sentinel: a rejected first candidate could previously take over best-tracking and the starting suffix stayed paired with a fake loss. log() caps infinite seeds for readability only. - Clear last_schedule_state before _update_attack_log_params so a rerun that fails during logfile setup no longer exposes the previous run's state. - Gate phase admissions, sentinel resets, and the control-weight bump on remaining step budget: exact-budget exhaustion now returns instead of tripping the carried-loss assertion (e.g. two progressive goals with n_steps=3 and an inner result of ("ctrl", 0.75, 3)). Finalize-on-success semantics are unchanged. Signed-off-by: fei <204683769+feiiiiii5@users.noreply.github.com>
Two sessions independently addressed romanlutz's round-two review on microsoft#2416. This merge keeps the transition-gating direction the reviewer suggested (skip admissions and sentinel resets once steps_completed >= n_steps) over relaxing the carried-loss assertion: gating keeps ``last_schedule_state`` honest (no admitted-but-never- scored phase recorded with an inf loss) and preserves the assertion's purpose of catching silent carry-over regressions. The duplicate regression test is superseded by exact-budget tests covering goal, worker, and control-weight transitions. Signed-off-by: fei <204683769+feiiiiii5@users.noreply.github.com>
|
Follow-up to the comment above: while preparing these fixes we consolidated two parallel work attempts on this branch. The budget-exhaustion case is now handled by gating the phase admissions and their sentinel resets on remaining step budget (the direction suggested in review), rather than relaxing the carried-loss assertion — see c8fd9a4. Gating keeps |
| # The inner run must have produced a measurable loss whenever any | ||
| # optimization happened; guards against silent carry-over regressions. | ||
| if schedule.steps_completed > 0: | ||
| assert not math.isinf(schedule.loss), "schedule.loss was never updated by the inner run" |
There was a problem hiding this comment.
This assertion uses inf to mean "the inner run never updated the loss," but a completed inner run can also report inf because of a non-finite model loss or numeric overflow. That case previously returned normally and now raises AssertionError. Could we track whether an inner result was received explicitly, or validate non-finite losses with a deliberate error at the source, rather than infer update state from the numeric value?
Purpose
Implements the structural refactor requested in #2416: a typed optimization-iteration state for the GCG loop, with the iteration lifecycle made explicit — while keeping public attack behavior and extension protocols unchanged.
Closes #2416
What changed
Typed state (
attack_manager.py)StopReasonenum:MAX_STEPS_REACHED,ALL_PROMPTS_JAILBROKENOptimizationRunStatedataclass: current suffix, best suffix, current/best loss, step counter, step runtime, stop reason. Exposed asMultiPromptAttack.last_run_stateafterrun(), so callers and tests can programmatically inspect why/when optimization stoppedProgressiveScheduleStatedataclass: goals/workers admitted so far, shared step counter, loss carry-over, inner stop flag. Exposed asProgressiveMultiPromptAttack.last_schedule_stateExplicit phases with stable contracts
MultiPromptAttack.run: stopping phase extracted to_all_training_prompts_jailbroken(); periodic logging phase extracted to_log_best_checkpoint()(same best-suffix swap-and-restore semantics as before, now with afinallyguarantee)ProgressiveMultiPromptAttack.run: scheduling counters moved intoProgressiveScheduleState; final evaluation extracted to_finalize_progressive_run()GCGMultiPromptAttack.step: selection phase (flat argmin → group/in-batch decomposition) extracted to_select_best_candidate(). Candidate batches intentionally remain step-local to preserve the existing VRAM-bounding behavior noted in the loop commentCompatibility
run()still returns(control, loss, steps)/(control, steps)); the new attributes are additiveValidation
tests/unit/executor/promptgen/gcg/test_run_state.py(12 tests): stop-reason assignment (incl. that the final success check does not count as an executed step), best tracking under annealing rejection, periodic checkpoint restore contract, seeded identical trajectories, argmin decomposition across worker groups, progressive finalize path with call-through assertions on the inner attack logruff check+ruff format --checkclean with the repo-pinned ruff v0.16.0AI usage disclosure
Human verification: I reviewed the diff hunk-by-hunk against the pre-refactor control flow, confirmed the annealing acceptance rule, admission ordering, weight bump threshold (
<= 0.09), and return semantics are preserved, and re-derived the argmin index decomposition used in the selection tests.