-
Notifications
You must be signed in to change notification settings - Fork 82
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
Urbis Mystic Implementation #1321
Conversation
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
…s swapping Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
…added Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
…n-closure issue Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
e33134d
to
c2400b0
Compare
Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
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.
Thanks a lot for the PR! I added a few comments below, could you please make those changes?
setup.py
Outdated
@@ -66,6 +66,8 @@ | |||
|
|||
extras_require = { | |||
"full": [ | |||
"scikit-learn>=1.0.0,<=1.2.0", |
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 this line should not be needed.
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.
Addressed!
lale/lib/aif360/urbis.py
Outdated
@@ -84,11 +101,17 @@ def fit(self, X, y): | |||
lambda row: "".join([str(v) for v in row]), axis=1 | |||
) | |||
assert X.shape[0] == group_and_y.shape[0] | |||
# TODO: Figure out a better workaround |
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.
After running ProtectedAttributeEncoder, I think should be able to unconditionally assume that the favorable labels are set([1])
. However, I would not overwrite self.favorable_labels
, because that would change the state of the object for future calls. Instead, why not just pass set([1])
into the _pick_sizes
call below?
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 offline, I'm using LabelEncoder
to encode the features separately and then construct a new set to pass to _pick_sizes
by using the encoder's mapping and the classes in favorable_labels
.
@@ -57,6 +70,8 @@ def __init__( | |||
estimator, | |||
unfavorable_labels=None, | |||
redact=True, | |||
imbalance_repair_level=0.8, |
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.
In addition to adding the hyperparameters to the constructor, we'll also need them in the _hyperparams_schema
.
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.
Addressed!
…otectedAttributesEncoder) Signed-off-by: Michael Feffer <mfeffer@Michaels-MacBook-Pro.local>
Using
mystic
solver from corresponding Python package to implement_pick_sizes
for downsampling via Urbis. Also wrote a few unit tests to evaluate functionality and some utility functions that can be extended for upsampling via Orbis and (potentially) mixed sampling (i.e., both upsampling and downsampling).