-
Notifications
You must be signed in to change notification settings - Fork 4
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
Improve memory usage of MetaLearnerGridSearch
#62
Merged
Merged
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
b6c3dd0
Add options for storing
FrancescMartiEscofetQC e8e3b39
Tests
FrancescMartiEscofetQC 44fa6ec
Finish TODO
FrancescMartiEscofetQC 629999d
Reduce memory usage by not creating metalearner object
FrancescMartiEscofetQC 6da4180
Update CHANGELOG
FrancescMartiEscofetQC 1e945ba
Use generator_unordered
FrancescMartiEscofetQC 4682e53
Add grid_size_ and move attributes initialization to fit
FrancescMartiEscofetQC b1fd5b8
Fix
FrancescMartiEscofetQC 4ef8e6e
Fix
FrancescMartiEscofetQC 950dda3
grid_size_ docstring
FrancescMartiEscofetQC 46a88cc
Add new options to tutorial
FrancescMartiEscofetQC e34b751
Remove check empty generator
FrancescMartiEscofetQC 62eacd4
Merge branch 'main' into generator_grid_search
FrancescMartiEscofetQC 7c16b02
Merge branch 'main' into generator_grid_search
FrancescMartiEscofetQC 4c1c12d
Apply suggestions from code review
FrancescMartiEscofetQC 273b864
Merge branch 'main' into generator_grid_search
FrancescMartiEscofetQC 1590e94
Add explanation grid_size_
FrancescMartiEscofetQC File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't quite grasped yet what the motivation for this attribute is. If the user expects a generator, won't they just be used to querying it until it no longer yields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! But I think it may be useful in the case the user wants to show a progress bar or similar, this way it's easier for them to access the number of metalearners to fit instead of having to calculate it manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand what you mean. At the same time this use case is not super clear to me. Shall we simplify and remove it until we witness that there is a need for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I may not explain it correctly, in the case we set
store_raw_results = False
andstore_results = False
then thefit
method finishes "instantly" (it does not wait for fitting the individual metalearners). Then, afaict these are only fitted when the user requests them by iterating over the generator where it may be of use to use thisgrid_size_
to display some progress bar or similar. If you still think it's not clear lmk and we can discuss it further.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, I added some explanation about this in the docstring:
1590e94