4569 consistent rng for data loader

[wyli]

Signed-off-by: Wenqi Li [email protected]

Fixes #4569

Description

with different num_workers the batch sampler's rng should be consistent, this PR keeps the initial random seed to achieve it.

Status

Ready

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change). # the previous results for num_wokers==0 might change
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[Nic-Ma]

Thanks for the update, looks good to me.

[wyli]

/build

[atbenmurray]

@wyli, this breaks any scenarios where the user has set up their RNG on their pipeline ahead of time. I understand the need for such a mechanism but I suggest reassessing it and looking at ways of making it opt-in

[wyli]

@wyli, this breaks any scenarios where the user has set up their RNG on their pipeline ahead of time. I understand the need for such a mechanism but I suggest reassessing it and looking at ways of making it opt-in

it's possible to use a predefined RNG as a keyword argument generator=_g:

MONAI/tests/test_integration_lazy_samples.py

Lines 106 to 110 in 9ee1b37

	_g = torch.Generator()
	_g.manual_seed(0)
	set_determinism(0)
	train_loader = monai.data.DataLoader(
	train_ds, batch_size=1, shuffle=True, num_workers=num_workers, generator=_g, persistent_workers=num_workers > 0

[atbenmurray]

@wyli, this breaks any scenarios where the user has set up their RNG on their pipeline ahead of time. I understand the need for such a mechanism but I suggest reassessing it and looking at ways of making it opt-in

it's possible to use a predefined RNG as a keyword argument generator=_g:

MONAI/tests/test_integration_lazy_samples.py

Lines 106 to 110 in 9ee1b37

	_g = torch.Generator()
	_g.manual_seed(0)
	set_determinism(0)
	train_loader = monai.data.DataLoader(
	train_ds, batch_size=1, shuffle=True, num_workers=num_workers, generator=_g, persistent_workers=num_workers > 0

That only works for scenarios where the RNG hasn't already been set up. I definitely think that there needs to be an option where it just doesn't alter the pipeline random state. In my case, I have added a suppress_rng flag just to get around the problem for the LR paper; it might be a good way to go if we make it a little bit semantically clearer what this flag is for, but is there a reason why the whole mechanism isn't opt-in?
Looking at the original comment on the PR, it seems like this could be more of a point fix for the batchsampler itself, no? It is the batchsampler rng that must be consistent across workers, yes?