Skip to content

implement core replay buffer logic - #792

Open
GHOpenonic wants to merge 2 commits into
mainfrom
replay-core
Open

implement core replay buffer logic#792
GHOpenonic wants to merge 2 commits into
mainfrom
replay-core

Conversation

@GHOpenonic

Copy link
Copy Markdown
Collaborator

add replay.py script and test. This does not add replay buffer as a training option, it only implements the core logic. Wiring it into training should be based on the LLC data munging stack #670, #671, #672, #673

@jder

jder commented Jul 20, 2026

Copy link
Copy Markdown
Member

@GHOpenonic can you point me to where this is used in your code? It's a bit hard to understand how these pieces are put together from this alone.

@GHOpenonic

Copy link
Copy Markdown
Collaborator Author

@jder most of it is used/further developed in datasets.py, config.py, base.py, stepper.py, and train.py in replay buffer agent attempt.
replay.py is updated and with implementations in datasets.py and train.py in slow replay buffer
I increase replay's dataloading/handling speed to be roughly gpu-compute bound by changing datasets.py and train.py in improve replay buffer train speed --> gpu bound.
Additionally, if you're curious, I make further changes to all these scripts and replay.py in the Jul 19, 2026 commits in llc_cpu_working, mostly shardtensor implementation phase 3: replay buffer, to get shardtensor to work with replay buffer training

@jder
jder self-requested a review July 21, 2026 13:57
@jder

jder commented Jul 21, 2026

Copy link
Copy Markdown
Member

ok, thanks. I think unless someone needs this sooner (@amogh-gulati maybe?) I vote we stage this after we land the LLC work #670 and rust data loader #800, since building this on top of those probably makes sense.

@amogh-gulati

Copy link
Copy Markdown
Collaborator

ok, thanks. I think unless someone needs this sooner (@amogh-gulati maybe?) I vote we stage this after we land the LLC work #670 and rust data loader #800, since building this on top of those probably makes sense.

I could work on the branch meanwhile

Comment thread src/samudra/replay.py

@dataclasses.dataclass
class ReplayEntry:
state: torch.Tensor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any way we could get more type information for what the state is, maybe via jaxtyping?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

e.g. is this a Prognostic?

Comment thread src/samudra/replay.py
stride: int
temporal_stride: int

def advance(self) -> "ReplayCursor":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
def advance(self) -> "ReplayCursor":
def advance(self) -> Self:

Comment thread src/samudra/replay.py
Comment on lines +27 to +37
@dataclasses.dataclass(frozen=True)
class ReplayBatchSlot:
replay_index: int
cursor: ReplayCursor


@dataclasses.dataclass(frozen=True)
class ReplaySeedSlot:
replay_index: int
cursor: ReplayCursor
reason: str

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐑 IIUC these are related concepts. Maybe the seed slot could inherent from the batch slot, and then add the one additional variable? Then, I wonder if we could use a single tuple instead of two tuples in the ReplayBatchRequests (targeting the base class).

Comment thread src/samudra/replay.py
self.buffer_size = buffer_size
self.storage_dtype = storage_dtype
self.generator = generator
self.pin_memory = pin_memory

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this used similar to a Batch object by torch (e.g. a TrainData)? If so, I would recommend adding to and pin_memory methods. If these two methods exists, the torch framework will try to call them as needed.

Comment thread src/samudra/replay.py
Comment on lines +112 to +126
)
if len(eligible) >= batch_size:
draw = torch.randperm(
len(eligible),
generator=self.generator,
device="cpu",
)[:batch_size]
else:
draw = torch.randint(
len(eligible),
(batch_size,),
generator=self.generator,
device="cpu",
)
return [eligible[int(i)] for i in draw]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible that sample_indicies could call random_indices? Or, can this block (which seems to be shared with that method below) be extracted into a common, private method?

Comment thread src/samudra/replay.py
entry.ready_event.synchronize()
return entry.state.cpu()

def _prepare_entry(self, entry: ReplayEntry) -> ReplayEntry:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

4 participants