GAIL и GRPO: seed= не значил ничего - #46
Open
DenisDrobyshev wants to merge 2 commits into
Open
DenisDrobyshev wants to merge 2 commits into
DenisDrobyshev wants to merge 2 commits into
Conversation
Three fresh processes, same seed, same machine:
421.70 496.20 385.10
and two calls inside one process gave 185.3 and 442.9. `test_gail_imitates_expert`
asserts a threshold on that, which is why it fails about one run in ten and has
been blocking every pull request here.
Neither source is the weights. Both are data, and both are invisible to
`set_seed` for the reason its own docstring gives -- it cannot reach a generator
an object built for itself:
* the policy rollouts in `_collect_policy_transitions` start from `self.env`,
and an env owns an `np.random.default_rng()` with no seed, so every
iteration of `learn` drew its starting states from OS entropy;
* the policy dataset the discriminator trains against was rebuilt each
iteration as `TransitionDataset(...)` with no seed, so its minibatch indices
came from OS entropy too, on every discriminator epoch of every iteration.
Seeding the env once in `__init__` is enough for the first: `reset()` keeps the
generator it was handed, so the unseeded resets that follow draw from a seeded
stream. The dataset takes a seed drawn from this agent's own generator, so it
still differs per iteration and still reproduces.
Both are necessary and neither is sufficient. Measured, two runs in one process
at the same seed: both fixes 128.8 and 128.8; env seeding reverted 124.9 and
154.8; dataset seeding reverted 120.0 and 75.6. At the real test's parameters
three fresh processes now return 380.0, 380.0, 380.0 against a threshold of 200.
The new test is sized to catch both -- the first version of it was short enough
to pass with the env fix reverted, which would have been a regression test that
did not regress. It calls `set_seed` per run, matching what conftest does per
test, because that is the contract as it stands: an agent's networks are
initialised from global torch state rather than from its own seed, here and in
every other algorithm in this package. Two constructions of GAILDiscriminator in
one process give different checksums; after `set_seed(0)` they match. So `seed=`
means "reproducible given the same global state" and not yet "reproducible".
Making it mean the second is a change to every agent and belongs in its own
decision, not smuggled in here -- but it is the remaining half of #15.
GRPO(seed=0) дважды: 91.2 94.7 `_rollout_episode` resets the env once per episode and never with a seed, and `learn` never seeds it either, so every episode started from OS entropy. Same cause as GAIL's rollouts, same reason `set_seed` cannot help: an env owns an `np.random.default_rng()` built without a seed, which its docstring says. Seeded once at the top of `learn`, which is where `off_policy`, `tabular` and `sac_discrete` already do it. A reset keeps the generator it was handed, so the per-episode resets that follow draw from a seeded stream. Two runs now return 145.5 and 145.5. I audited the rest rather than assuming. Ten algorithms reset inside their training loop; eight of them -- dqn, dreamer, mbpo, off_policy, reinforce, rssm, sac_discrete, tabular -- seed the first reset, and the on-policy base and recurrent_ppo seed their vector env the same way. GRPO was the only one left, and `decision_transformer` and `her` seed per episode deliberately. So this is two cases in the package, not a pattern still hiding elsewhere. The test fails with this line reverted, which is the only reason to believe it.
DenisDrobyshev
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Три свежих процесса, один сид, одна машина:
Два вызова внутри одного процесса — 185.3 и 442.9.
test_gail_imitates_expertутверждает порог на этом, поэтому падает примерно раз в десять прогонов и держит каждый PR в репозитории.Ни один источник — не веса
Оба — данные, и оба невидимы для
set_seedровно по той причине, которую называет его собственный докстринг: он не достаёт до генератора, который объект построил себе сам.Траектории политики в
_collect_policy_transitionsстартуют изself.env, а среда владеетnp.random.default_rng()без сида. Каждая итерацияlearnбрала начальные состояния из энтропии ОС.Датасет политики, против которого учится дискриминатор, пересоздавался каждую итерацию как
TransitionDataset(...)без сида — то есть индексы минибатчей тоже шли из энтропии ОС, на каждой эпохе дискриминатора каждой итерации.Сид среды ставится один раз в
__init__:reset()сохраняет переданный генератор, так что последующие несеянные сбросы тянут из засеянного потока. Датасет получает сид из собственного генератора агента — он по-прежнему разный на каждой итерации и по-прежнему воспроизводится.Обе необходимы, ни одна не достаточна
Два прогона в одном процессе, один сид:
На параметрах настоящего теста три свежих процесса дают 380.0, 380.0, 380.0 при пороге 200.
Про сам тест
Он размерен так, чтобы ловить обе правки. Первая версия была слишком короткой и проходила с откаченным сидом среды — то есть была бы регрессионным тестом, который ничего не регрессирует. Проверено: с любой из двух правок откаченной он падает, с обеими проходит.
set_seedзовётся на каждый прогон — это то, что conftest делает на каждый тест, и это текущий контракт: сети агента инициализируются из глобального состояния torch, а не из собственного сида — здесь и во всех остальных алгоритмах пакета.То есть
seed=сейчас значит «воспроизводимо при том же глобальном состоянии», а не «воспроизводимо». Сделать второе — это изменение во всех агентах, отдельное решение, и протаскивать его сюда неправильно. Но это оставшаяся половина #15.Что это меняет для #15
Я до этого предлагал статистический обход — медиану пяти сидов. Он больше не нужен для GAIL: у теста просто не работал сид, и починка сида лучше, чем обкладывание статистикой невоспроизводимого прогона. Стоит проверить остальные 24 одноразовых assert'а на то же самое, прежде чем менять протокол.
И то же самое нашлось в GRPO
_rollout_episodeсбрасывает среду раз в эпизод и никогда с сидом,learnеё тоже не сеет. Та же причина, тот жеset_seed, который до этого не достаёт.Засеяно один раз в начале
learn— там, где это уже делаютoff_policy,tabularиsac_discrete. Два прогона теперь дают 145.5 и 145.5. Тест падает с откаченной строкой.Остальные я проверил, а не предположил
Десять алгоритмов сбрасывают среду внутри цикла обучения. Восемь из них —
dqn,dreamer,mbpo,off_policy,reinforce,rssm,sac_discrete,tabular— сеют первый сброс, и несеянные сбросы внутри цикла тянут из засеянного потока. On-policy база иrecurrent_ppoтак же сеют свою векторную среду.decision_transformerиherсеют поэпизодно намеренно.GRPO был единственным оставшимся. То есть это два случая в пакете, а не схема, которая где-то ещё прячется.