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.
[ADD] Minority Coalescer #242
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
[ADD] Minority Coalescer #242
Changes from all commits
d85c16e
ae0652e
90c71eb
43213bb
e107e93
e40ed30
3d25f37
2e08e99
c8aa626
afd4f00
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
minimum_fraction
->min_fraction
(convention)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 don't think we have a convention for this, and in this case, having the complete word is more clear. If possible I would like to preserve 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.
Pytorch uses
min
rather thanminimum
.When it uses
minimum
, it is only for the element-wise minimum.But if you would like to stick to it, it is also fine.
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.
Why?
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.
This is a
NoCoalescer
class. This allows the BO model to enable/disable coalescing.The choice object selects between MinorityCoalescer and NoCoalescer depending on what gives better performance.
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 mean i did not get if you mean
Do not perform NO coalescer
orDo not perform coalescer
.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.
What is
X
dictionary?fit_dictionary
? (Ravin says thefit_dictionary
will deprecate soon, but do you have any idea when?)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.
Scikit-learn supports passing a dictionary alongside the data. See here
It makes a lot of sense to use it instead of X as a fit_dictionary.
From all of the refactoring changes, this is to me the most important.
When** depends on when there is a contributor that wants to do this change :)
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.
Sorry I did not get you, so please add your ideas to the doc-string as well?
Especially, I do not get why the meaning behind 'X' in the sentence.
But still it is a bit confusing for me.
Do you know why sklearn uses
X
for bothfit_dictionary
andfeature_data
?