Skip to content

Use a distinct seed per environment rebuild - #1007

Open
alexmillane wants to merge 4 commits into
alex/feature/move_to_15Hzfrom
alex/fix/per_rebuild_seeding
Open

Use a distinct seed per environment rebuild#1007
alexmillane wants to merge 4 commits into
alex/feature/move_to_15Hzfrom
alex/fix/per_rebuild_seeding

Conversation

@alexmillane

Copy link
Copy Markdown
Collaborator

Summary

Distinct env-builder seed per rebuild

Detailed description

  • Reason: Every rebuild constructed the environment with the same seed, so build-time variations (e.g. directional-light direction) resolved to the same value on each rebuild, defeating the purpose of rebuilding.
  • Change: Added _cfg_for_rebuild, which offsets environment_builder.seed by the rebuild index. Rebuild 0 keeps the configured seed so single-rebuild runs are unchanged, and the original config is never mutated (a fresh copy via dataclasses.replace).
  • Impact: Multi-rebuild runs now see genuinely different build-time variation draws across rebuilds; single-rebuild behaviour is untouched. Covered by a new unit test plus an updated budget-split test.

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self review.

Comment on lines +89 to +90
# Rebuild 0 builds the run unchanged; each later rebuild offsets the seed so its fresh
# construction differs (e.g. a build-time light-direction variation resolves to a new value).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorten to. Runs are the same except for their seeds.

Comment on lines +91 to +94
assert received_run_cfgs[0] is run
assert [cfg.environment_builder.seed for cfg in received_run_cfgs] == [base_seed, base_seed + 1]
assert run.rollout_limit == RolloutLimitCfg(num_episodes=5)
assert run.environment_builder.seed == base_seed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually test that the second run is the same except for the seed. Consider copying run, altering the seed and testing:

assert received_run_cfgs == [run_seed_1, run_seed_2]

@alexmillane
alexmillane changed the base branch from main to alex/feature/move_to_15Hz August 4, 2026 09:48
@alexmillane
alexmillane marked this pull request as ready for review August 4, 2026 09:54
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review. (163 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

@alexmillane
alexmillane force-pushed the alex/fix/per_rebuild_seeding branch from ffbcf70 to 97d5476 Compare August 4, 2026 10:02

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review 2

Comment on lines +136 to +138
Without this every rebuild constructs with the same seed, so build-time variations
(e.g. the directional-light direction) resolve to the same value on each rebuild.
Rebuild 0 keeps the configured seed (``seed + 0``) so single-rebuild runs are unchanged.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the details here.

@alexmillane
alexmillane force-pushed the alex/fix/per_rebuild_seeding branch from 21f144a to 89b7372 Compare August 4, 2026 11:39
Offset the environment-builder seed by the rebuild index so each fresh
construction differs. Without this every rebuild constructs with the same
seed, so build-time variations (e.g. directional-light direction) resolve
to the same value on each rebuild. Rebuild 0 keeps the configured seed so
single-rebuild runs are unchanged, and the original config is never mutated.

Signed-off-by: alex <amillane@nvidia.com>
@alexmillane
alexmillane force-pushed the alex/fix/per_rebuild_seeding branch from 89b7372 to 6c4caea Compare August 4, 2026 11:42

@cvolkcvolk cvolkcvolk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

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.

2 participants