-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
StatefulDataloader shuffling ignores system RNGs at initiation, in fact internal random state is identical for all (?) new dataloaders #1440
Comments
Hi @gailweiss Let me take a deeper look into the issue. from torch.utils.data import RandomSampler
from torchdata.stateful_dataloader import StatefulDataLoader as DataLoader
def get_dl(d, generator):
sampler = RandomSampler(d, generator=generator)
return DataLoader(d, batch_size=1, sampler=sampler)
def same_order(dl1, dl2):
order1 = [b.item() for b in dl1]
order2 = [b.item() for b in dl2]
print("order1", order1)
print("order2", order2)
assert len(order1) > 0 # not accidentally on an empty one
return order1 == order2
seed = 0
generator = torch.Generator()
generator.manual_seed(seed)
dl1 = get_dl(list(range(10)), generator)
seed = 1
generator = (
torch.Generator()
) # You can create a new generator or can also use the old one
generator.manual_seed(seed)
dl2 = get_dl(list(range(10)), generator)
print(
"new dataloaders (started with different generators) on same dataset create same order?: ",
same_order(dl1, dl2),
) output:
Let me know if this works for you. |
Dear @ramanishsingh , thanks for looking into this! Your solution seems to work, thank you! If I may suggest, I found I can even fix the issue by using the
output:
It seems in this case that simply adding these lines to the
|
🚀 The feature
Brief description
The random generator in newly created StatefulDataLoaders should be randomly initiated - but it is currently deterministic. This will make different DataLoaders generate different shuffles of the data, more in line with how 'normal' DataLoaders behave. Such a change is, to my understanding, not in conflict with the statefulness of the dataloaders (the generator can still be stored and loaded, there's just no reason for it to start the same way each time)
Current state
At the moment, all statefuldataloaders with shuffling on have the same initial internal RNG state, regardless of actual environment RNG states at the initiation of these two dataloaders. For example:
Yields:
This behaviour does not seem necessary for the "statefulness" of the dataloaders, as their state_dict contains a tensor controlling the shuffles, so whichever random state they currently have can be saved and loaded as needed: new ones don't all need to start from the same random state.
Request
I normally expect new, shuffling, dataloaders to create a different shuffles from each other, and in particular to be sensitive to the environment's random state at initiation (and I assume I have this type of variety when testing runs on different random seeds).
I think this can be remedied by setting the generator tensor (
dl.state_dict()["_index_sampler_state"]["sampler_iter_state"]["generator"]
) randomly on initiation. I would use a workaround, something like this, if only I understood what the generator tensor is composed of and how to make a legal one:I would appreciate a "correct" version of the code in
shuffling_dl_getter
above being added to the initiation of theStatefulDataLoader
s! Unfortunately I don't understand the composition of the generator tensor so I can't build a 'good' one myself. In particular I notice that g is a tensor of length 5056 with many 0s and many higher numbers, while mt19937 states should be of length 624, and I don't know what all the extra stuff isMotivation, pitch
When testing sensitivity of an architecture or training routine to random state, I assume that the data order is being changed too (and not just the network's initial weights, and dropouts throughout training)
Alternatives
If I could, I would use the
shuffling_dl_getter
code described above to obtain randomly initiated StatefulDataLoaders myself, unfortunately, it is not clear to me how to make legal random states for the dataloadersAdditional context
No response
The text was updated successfully, but these errors were encountered: