Add solution for explicit randomization in subprocesses.

[kosmitive]

Description

This PR closes #242, #392 and #398

Changes

• Added seed to functions from pydvl.utils.numeric, pydvl.value.shapley and
  pydvl.value.semivalues.
• Added new tests for testing reproducibility of modified functions.
• Introduced new type Seed and conversion function ensure_seed_sequence.

Postponed

• Randomization for PyTorch dependent algorithms and modules Explicit seeding for pyTorch #397.
• Full reproducibility due to random order of execution Introduce history into executor pattern for reproducibility. #416.

[AnesBenmerzoug]

This isn't the recommended way to implement seeded random number generation. We should favour creating rng objects (in the case of numpy) and a Random object (in the case of pure Python starting from version 3.9) and use those for any subsequent generation.

Here's how it's done in scikit-learn: https://github.com/scikit-learn/scikit-learn/blob/2a2772a87b6c772dc3b8292bcffb990ce27515a8/sklearn/utils/validation.py#L1207

We could use that as inspiration, even though it's using the legacy RandomState class instead of the new Generator class.

This doesn't really close #242. To solve that we should expose a seed parameter in all algorithms that we can use to seed the different workers.

Refer to this and this for more details.

[kosmitive]

@AnesBenmerzoug

1. Can we drop support for Python 3.8. Reason for that is that in numpy>=1.25, the Generator object has a spawn method. This method doesn't exist in previous versions. However Python 3.8 doesn't requires numpy<1.25. Not having this method makes the design for seeding more complex as one has to distinguish between SeedSequence and Generator.

2. I need to adapt the tests as they also rely on seeding and fix bugs in the seeding before I request a review.

3. Do we want to create another issue for pyTorch to keep this PR small?

[AnesBenmerzoug]

@kosmitive

1. No, we shouldn't drop Python 3.8 until it reaches its end-of-life (14 Oct 2024). The Generator's spawn is just a convenience that was added in numpy 1.25 and we shouldn't bump the minimum requirement just for that. We can just directly instantiate a SeedSequence object and call its spawn method.

2. Yes, please do that so we can be sure that this change works. No need to run all tests twice though to check that seeding works.

3. Yes, that makes sense. Anyway, the random seed handling is different in PyTorch.

[kosmitive]

@kosmitive

1. No, we shouldn't drop Python 3.8 until it reaches its end-of-life (14 Oct 2024). The `Generator`'s `spawn` is just a convenience that was added in numpy 1.25 and we shouldn't bump the minimum requirement just for that. We can just directly instantiate a [SeedSequence](https://numpy.org/doc/stable/reference/random/bit_generators/generated/numpy.random.SeedSequence.html#numpy.random.SeedSequence) object and call its `spawn` method.

2. Yes, please do that so we can be sure that this change works. No need to run all tests twice though to check that seeding works.

3. Yes, that makes sense. Anyway, the random seed handling is different in PyTorch.

1. Okay then I allow only to pass a SeedSequence prior to using parallel execution.

2. I'll create an issue for further investigation.

[mdbenito]

Thanks for the PR. I have left a couple of comments, but I think we need to discuss the general design for this. It is important to distinguish the different concepts used and how they enter the picture at which stages.

[kosmitive]

@mdbenito @AnesBenmerzoug Let's decide on one way. Do we want to go forward with a seed parameter approach or do we switch to a different approach. What are your thoughts?

[mdbenito]

@kosmitive This error indicates that you must have made some mistake merging develop into your branch, because the function semivalues was removed from that branch and its tests.

[AnesBenmerzoug]

What is the logic behind this change?

[kosmitive]

Once a partial function is passed to maybe_add_argument, the inner __call__ logic is neglected for these types and thus also if it is contained in the arguments. Furthermore partial has to be used, because otherwise the wrapped function can't be pickled (happens for seed but not for job_id). So recursive partial support is needed in the checking.

[mdbenito]

More comments. Please double check all docstrings before requesting another review.

[mdbenito]

@kosmitive I went ahead and pushed directly to your branch because I don't know how to explain better that you need to think of the readers when writing documentation. I also moved things around a bit and made a couple minor changes / simplifications.