FIX Propagate GCG random_seed to all RNG sources for deterministic runs - #2502
FIX Propagate GCG random_seed to all RNG sources for deterministic runs#2502Amruth Vamshi (AmruthVamshi) wants to merge 19 commits into
Conversation
|
@microsoft-github-policy-service agree |
| py_rng = random.Random(random_seed) | ||
| models = getattr(self, "models", None) | ||
| device = models[0].device if models else "cpu" | ||
| self._torch_gen = torch.Generator(device=device).manual_seed(random_seed) |
There was a problem hiding this comment.
Could we create this RNG state once for the complete GCG execution instead of once per inner MultiPromptAttack.run()? Progressive and individual attacks invoke this method repeatedly with the same random_seed, so every goal/model phase restarts both random streams. For example, two progressive phases using seed 42 both begin with the same first Torch draw rather than continuing one deterministic trajectory.
Please own the RNG bundle at the outer execution level, derive stable child streams per worker/device, and pass those streams into inner attacks. The base and derived seeds should also be recorded in run metadata as required by #2490.
There was a problem hiding this comment.
Done. Created an RngBundle dataclass in generator.py that holds a local random.Random, numpy.random.Generator, and per-worker torch.Generator instances (derived as seed + i per worker index). The bundle is created once in _setup_async after workers are spawned, stored on GCGContext, and threaded through Progressive/Individual attacks into each inner MultiPromptAttack.run() via attribute injection. Inner runs reuse the same RNG instances rather than restarting from the seed. Base seed and derived worker seeds are now recorded in the JSON log params as "random_seed" and "derived_seeds".
There was a problem hiding this comment.
Thanks. The GCGGenerator path now shares one bundle and the metadata part looks addressed, but I want to keep this open for the remaining execution paths.
Direct ProgressiveMultiPromptAttack and IndividualPromptAttack runs still have no outer bundle to forward, so each inner attack rebuilds its RNGs from the same seed and restarts the stream. Please have those outer attacks create and own the bundle when one was not injected, then reuse it for every inner run.
Please also cover real concurrent executions across NumPy augmentation, Torch sampling, and Python annealing. The current concurrency test only interleaves standalone NumPy calls sequentially. A direct progressive/individual regression should show consecutive inner phases consume different draws while repeating the complete run reproduces the same sequence.
There was a problem hiding this comment.
Done. ProgressiveMultiPromptAttack.run() and IndividualPromptAttack.run() now create and own an RngBundle when none was injected, so inner MultiPromptAttack.run() calls reuse the same streams instead of restarting from the base seed each phase.
The concurrency test (test_concurrent_runs_isolated_across_all_rng_types) goes through a real ProgressiveMultiPromptAttack with two goals (two progressive phases). Each inner phase draws from all three bundle streams, np_rng, torch_gens[0], and py_rng, and the test asserts that:
- Consecutive phases see different draws (streams advanced, not restarted) and
- Repeating the full run with the same seed reproduces the exact sequence.
There was a problem hiding this comment.
I verified that direct progressive and individual attacks now create one bundle and reuse it across inner phases. The claimed concurrency coverage is still not present, though: test_concurrent_runs_isolated_across_all_rng_types runs the two complete executions one after another in a for loop, with a mocked inner attack. It never overlaps them, and it does not cover the individual or progressive-model paths or complete trajectories.
I also reproduced a concrete interference case that this misses: two concurrent generators using the same/default result prefix receive the same logfile path because it has one-second resolution, and both attacks use unlocked read/modify/write logging. Please make per-run output paths unique, run the executions with real overlap, and assert each run's candidates, accepted controls, loss history, and final suffix match its isolated baseline across the topologies required by #2490. I have staged a current-line comment with the reproduction details.
There was a problem hiding this comment.
You're right, that test wasn't testing overlap at all, and thanks for the repro on the logfile path. I hadn't realised the two runs were landing on the same file.
I went with making the path unique rather than adding a lock. _build_logfile_path now appends a short random id, so the file ends up as {result_prefix}_{YYYYMMDD-HHMMSS}_{id}.json, and an explicit logfile is passed through untouched. A lock wouldn't have been enough here since the file doubles as the result channel: _initialize_attack_log truncates it, log() appends to the shared arrays, and _read_result reads controls[-1] back, so two runs on one path would still merge even if every write were serialised. Docstring and the AML notebook are updated. If you'd rather keep the old shape with a pid or counter instead of the random id, happy to change it.
And no, there wasn't an isolation mechanism elsewhere, that was the bug.
For the tests I dropped the sequential one and replaced it with real end-to-end runs. There's a small trajectory_stubs.py next to the tests with a worker and prompt manager whose gradients and logits are pure functions of their inputs, so the real IndividualPromptAttack / ProgressiveMultiPromptAttack drive the real GCGMultiPromptAttack.step() with the bundle as the only source of randomness. test_overlapping_runs_reproduce_isolated_trajectories runs over five topologies (individual 1w/1g and 1w/2g, progressive goals 1w/2g, progressive goals+models 2w/2g, plain multi 2w/2g). Two runs go in two threads with a threading. Barrier inside the worker's GRAD op so they're forced to alternate step by step, and the test checks the run-id sequence actually switched more than once. Each run is then compared against its isolated baseline on sampled candidates, the accepted control going into every step, the per-step losses, the JSON controls/losses and the final suffix. It also checks repeating seed 42 reproduces the baseline, 42 and 7 differ, and every inner phase saw the same RngBundle object.
TestConcurrentGenerators does the same one level up through execute_async with asyncio.gather and a shared result_prefix, for both the individual and progressive strategies, and checks the two runs got different log_paths and the same final_suffix / control_history / loss_history as when run alone. I also added test_augmentation_draws_from_the_run_seeded_numpy_stream so the NumPy side is covered too: it checks the generator handed to augmentation is default_rng(seed) advanced by one draw per target.
To make sure these are correct, I broke each stream on purpose: skipping the torch_generator pass-through fails 7 tests, swapping the annealing draw for module-global random.random() fails 5, and passing np_rng=None to augmentation fails 1.
…t#2490) Create a per-run RngBundle at the outer execution level (_setup_async) with local random.Random, numpy.random.Generator, and per-worker torch.Generator instances derived deterministically from the configured seed. Thread the bundle through Progressive/Individual attacks into inner MultiPromptAttack runs so that repeating a configuration produces identical candidates, annealing decisions, and final suffixes. Record base seed and derived worker seeds in run metadata JSON.
…ators, and exact acceptance tests (microsoft#2490)
| worker1.model.device = torch.device("cuda:1") | ||
| attack.workers = [worker0, worker1] | ||
|
|
||
| attack.run(n_steps=1, stop_on_success=False, anneal=False, random_seed=42) |
There was a problem hiding this comment.
This new regression fails in a CPU-only PyTorch build before the assertions. torch.Generator(device=torch.device("cuda:0")) raises RuntimeError: Cannot get CUDA generator without ATen_cuda library; the complete GCG unit suite is currently 252 passed, 1 failed.
Please make the test hardware-independent rather than catching this in production. For example, patch torch.Generator to record each requested device while returning a CPU generator, then assert both calls requested worker 0's sampling device. That verifies the placement logic without requiring CUDA.
There was a problem hiding this comment.
I only have a CUDA build locally so never hit this. I did what you suggested: torch.Generator in attack_manager is patched with side_effect=lambda device: real_generator(), so each call records the device it was asked for and hands back a CPU generator, and the test asserts both calls asked for cuda:0 and that two generators were stored. I ran the whole GCG suite on a CPU-only build (macOS torch, Python 3.14) where torch.Generator(device="cuda:0") raises the same ATen_cuda error you saw, and it's 263 passed there.
|
|
||
| all_runs: list[list[PhaseSnapshot]] = [] | ||
|
|
||
| for _ in range(2): |
There was a problem hiding this comment.
This does not exercise concurrent executions. The loop runs each progressive.run() to completion before creating the next one. More importantly, real overlap with the default output configuration is not isolated: _build_logfile_path() only includes seconds, so two runs started in the same second receive the same JSON path, and _initialize_attack_log() plus log() perform unlocked read/modify/write operations on that file. I reproduced two concurrent generators returning the same path. One run can therefore truncate or overwrite the other run's trajectory, and _read_result() can return the other run's result.
Please allocate a unique path per run and replace this loop with genuinely overlapping executions, for example a barrier-backed executor or asyncio.gather. The assertions should compare each run's sampled candidates, accepted controls, loss history, and final suffix against its isolated baseline. The acceptance criteria also call for multi-prompt, multi-model, progressive-goal, and progressive-model coverage. Am I missing an isolation mechanism elsewhere?
There was a problem hiding this comment.
Covered in the attack_manager.py thread above, same commit.
…logfile paths Address review round 3 on microsoft#2502: - GCGGenerator._build_logfile_path appends a short random id so concurrent runs sharing a result_prefix never read/modify/write the same JSON log. - Deterministic in-process worker/prompt-manager stubs drive the real IndividualPromptAttack / ProgressiveMultiPromptAttack and the real GCGMultiPromptAttack.step() end to end. - Overlapping runs (barrier-forced threads, asyncio.gather) across five topologies reproduce their isolated candidates, accepted controls, loss history and final suffix; all inner phases share one RNG bundle. - Target augmentation is asserted to consume the run-seeded NumPy stream. - CUDA placement test patches torch.Generator so it runs on CPU-only builds.
Description
Fixes #2490.
GCGAlgorithmConfig.random_seedonly seeded CSV row shuffling. The three RNG call sites in the optimization loop used unseeded global state, making runs non-reproducible even with the same seed.Replace global RNG calls with local seeded instances:
np.random.default_rng(seed)for target augmentationtorch.Generator(device=device).manual_seed(seed)for candidate samplingrandom.Random(seed)for annealing acceptanceThis ensures same seed = same results, concurrent runs are isolated, and custom extension points (via
SamplingStrategyprotocol) remain backward compatible.Builds on #2467 by upgrading its global
random.seed()stopgap to use the properrandom_seedparameter.Tests and Documentation
TestRandomSeedDeterminismclass (9 tests) intest_gcg_core.pycovering: determinism for each RNG source, different-seed divergence, concurrent isolation, torch generator threading, and custom sampler backward compatibility.test_run_state.pyto use therandom_seedparameter instead of globalrandom.seed()calls.random_seeddocstring already describes the intended behavior; this PR makes it truthful.Local GPU verification script
Local GPU verification output
GCG Determinism Test — GPU, GPT-2
[Test 1] Single-model, single-prompt — same seed (42)...
[transformers]
torch_dtypeis deprecated! Usedtypeinstead!Loading weights: 100%|████████████████| 148/148 [00:00<00:00, 2978.82it/s]
loss=3.1953: 100%|███████████████████| 1/1 [00:00<00:00, 2.51it/s]
loss=2.9805: 100%|███████████████████| 1/1 [00:00<00:00, 3.07it/s]
loss=2.8398: 100%|███████████████████| 1/1 [00:00<00:00, 3.23it/s]
Loading weights: 100%|████████████████| 148/148 [00:00<00:00, 2307.39it/s]
loss=3.1953: 100%|███████████████████| 1/1 [00:00<00:00, 3.09it/s]
loss=2.9805: 100%|███████████████████| 1/1 [00:00<00:00, 3.12it/s]
loss=2.8398: 100%|███████████████████| 1/1 [00:00<00:00, 3.32it/s]
Run 1: suffix='! ! ! ! ! ! !again ! !ational ! ! ! ! ! ! ! ! !' loss=2.8398
Run 2: suffix='! ! ! ! ! ! !again ! !ational ! ! ! ! ! ! ! ! !' loss=2.8398
PASS
[Test 2] Single-model, single-prompt — different seed (99)...
Loading weights: 100%|███████████████| 148/148 [00:00<00:00, 2337.70it/s]
loss=3.0156: 100%|██████████████████| 1/1 [00:00<00:00, 3.32it/s]
loss=2.8867: 100%|██████████████████| 1/1 [00:00<00:00, 3.18it/s]
loss=2.8184: 100%|██████████████████| 1/1 [00:00<00:00, 3.27it/s]
Run 3: suffix='! ! ! ! ! ! ! ! ! ! ! ! ! ! !mm !SG ! !' loss=2.8184
PASS