Jean/efficient bootstrapping #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeandut
commented
Jan 19, 2024
ghost
reviewed
Jan 25, 2024
|
|
||
| btst_algo = BtstAlgo(**strategy.algo.kwargs) | ||
|
|
||
| class BtstStrategy(strategy.__class__): |
There was a problem hiding this comment.
BootstrapStrategy would have been more explicit IMO (same for BootstrapAlgo vs BtstAlgo
Comment on lines
+189
to
+192
| self.checkpoints_list = [ | ||
| copy.deepcopy(self._get_state_to_save()) | ||
| for _ in range(len(bootstrap_seeds_list)) | ||
| ] |
There was a problem hiding this comment.
I don't understand this part. @jeandut could you please explain why you do not need to pass a variable to indicate which bootstrap replicate to save?
| datasamples.shape[0], replace=True, random_state=rng | ||
| ) | ||
| # Loading the correct state into the current main algo | ||
| if self.checkpoints_list[idx] is not None: |
There was a problem hiding this comment.
It must be not None, no? Otherwise, there is an issue?
Comment on lines
+375
to
+394
| def equality_check(a, b): | ||
| if type(a) != type(b): # noqa: E721 | ||
| return False | ||
| else: | ||
| if isinstance(a, dict): | ||
| for k in a.keys(): | ||
| if not equality_check(a[k], b[k]): | ||
| return False | ||
| return True | ||
| elif isinstance(a, list): | ||
| for i in range(len(a)): | ||
| if not equality_check(a[i], b[i]): | ||
| return False | ||
| return True | ||
| elif isinstance(a, np.ndarray): | ||
| return np.all(a == b) | ||
| elif isinstance(a, torch.Tensor): | ||
| return torch.all(a == b) | ||
| else: | ||
| return a == b |
There was a problem hiding this comment.
Putting this function outside would easy readability
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements a more efficient way to do bootstrap in FL using substra using variations of @arthurPignetOwkin's code patterns.
While the details of this implementation are quite tricky the main idea is simple: hook all methods of algo and strategies so that they are executed
n_bootstraptimes producingn_bootstrapoutputs. The rest is just boilerplate code to glue everything together respecting substra and substrafl's constraints.This PR is a first attempt and will probably be simplified over time (next iteration will use different self for the different bootstrap runs aka the self will not merged, which will allow much cleaner code and avoid tricky side effects).
Note that the local bootstrap vs global bootstrap is an issue but is a detail of the PR and will be done in another PR.