EqualityAI / EqualityML

Evidence-based tools and community collaboration to end algorithmic bias, one data scientist at a time.
Apache License 2.0
34 stars 3 forks source link

Correlation remover uses fit_transform() instead of fit() and transform() #10

Closed jaared closed 1 year ago

jaared commented 1 year ago

The training and testing sets should use the same correlation remover object. It should be fit() on only the training data, and should transform() both the training data and the testing data.

These lines of code show an example:

cr = CorrelationRemover(sensitive_feature_ids=['sex'], alpha=1)
cr.fit(train_data.drop(['two_year_recid'], axis=1))
train2 = cr.transform(train_data.drop(['two_year_recid'], axis=1))
test2 = cr.transform(testing_data.drop(['two_year_recid'], axis=1))

These are the relevant GitHub references for review.

https://github.com/EqualityAI/EqualityML/blob/90e04435007a653b5c4c69dc5a9b86e0c5d34ce7/equalityml/fair.py#L219-L244

https://github.com/EqualityAI/EqualityML/blob/90e04435007a653b5c4c69dc5a9b86e0c5d34ce7/equalityml/fair.py#L363-L371

Comment:

Disparate impact remover is coded to use only fit_transform(). AIF360 did not provide a transform() function https://github.com/Trusted-AI/AIF360/blob/master/aif360/algorithms/preprocessing/disparate_impact_remover.py. They say "In order to transform test data in the same manner as training data, the distributions of attributes conditioned on the protected attribute must be the same." We could technically make the same assumption and always use fit_transform() for correlation remover because it seems to perform well, but I think it is bad practice as in deployment sometimes we make only one prediction at a time and we can't estimate correlation when only one observation is present, fit_transform() will error out. This is a weakness of the disparate impact remover.

JoaoGranja commented 1 year ago

You are right. I tested the same object "cr" to transform the testing data but the results are worse.

jaared commented 1 year ago

As discussed here, cr should improve other metrics than statistical parity.

JoaoGranja commented 1 year ago

Fixed