Skip to content
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

Binary group fairness metrics #1404

Merged
merged 102 commits into from
Mar 4, 2023
Merged

Binary group fairness metrics #1404

merged 102 commits into from
Mar 4, 2023

Conversation

AndresAlgaba
Copy link
Contributor

@AndresAlgaba AndresAlgaba commented Dec 21, 2022

What does this PR do?

This PR initializes the addition of observational group fairness metrics. The general idea of these metrics is to compare the model's outputs across different groups created by the protected attribute(s) under evaluation.

As a first step, I implemented two common group fairness metrics, demographic parity and equal opportunity, for binary classification problems. For demographic parity, we compare the positivity rates and, for equal opportunity, the true positive rates. In the case of more than two groups, we use the largest disparity, i.e., dividing the lowest rate between the highest.

In this initial proposal, I build on the stat_scores and add some logic to compute the positivity and true positive rates for groups. There are still several issues, but I believe that this proposal allows for a more clear discussion. For example, I'm unsure whether the _binary_groups_stat_scores should be part of stat_scores or group_fairness. Overall, I tried to follow the style and logic of the other classification metrics as closely as possible.

I will also add documentation and appropriate testing at a later stage when the big decisions on the design of the API for group fairness metrics are settled. There are also several potential extensions, such as including additional classification metrics for detecting discrimination and adding a similar logic to regression-based metrics.

I leave it as a draft for now. @Borda

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@Borda Borda added this to the v0.12 milestone Dec 21, 2022
@Borda
Copy link
Member

Borda commented Dec 23, 2022

hi @stancld or @lucadiliello could you pls assist here with this PR?

Copy link
Contributor

@lucadiliello lucadiliello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thanks for your contribution! I added some suggestions to improve code speed. Since I solved a similar problem when grouping metrics by indexes, if you need help feel free to ask!

@codecov
Copy link

codecov bot commented Dec 23, 2022

Codecov Report

Merging #1404 (d0f7b17) into master (7821012) will increase coverage by 0%.
The diff coverage is 72%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #1404    +/-   ##
=======================================
  Coverage      88%     88%            
=======================================
  Files         223     225     +2     
  Lines       11708   11860   +152     
=======================================
+ Hits        10289   10466   +177     
+ Misses       1419    1394    -25     

@AndresAlgaba
Copy link
Contributor Author

I have added documentation.

@stancld
Copy link
Contributor

stancld commented Feb 28, 2023

@Borda @justusschock Do you have any clue why our tests failing with some old configurations? 🤔

@Borda
Copy link
Member

Borda commented Feb 28, 2023

@Borda @justusschock Do you have any clue why our tests failing with some old configurations? thinking

I think I remember this one, you are passing somewhere none instead of expected int:

TypeError: '<' not supported between instances of 'int' and 'NoneType'

I would suggest recreating this env and debugging locally... to do you you can use:
https://github.com/Lightning-AI/metrics/blob/50388cfb167ba323fcc407a89d83f2aec0dfb171/.github/actions/pull-caches/action.yml#L33

@mergify mergify bot added ready and removed ready labels Mar 1, 2023
auto-merge was automatically disabled March 1, 2023 23:51

Merge queue setting changed

@stancld
Copy link
Contributor

stancld commented Mar 2, 2023

@Borda @justusschock Do you have any clue why our tests failing with some old configurations? thinking

I think I remember this one, you are passing somewhere none instead of expected int:

TypeError: '<' not supported between instances of 'int' and 'NoneType'

I would suggest recreating this env and debugging locally... to do you you can use:

https://github.com/Lightning-AI/metrics/blob/50388cfb167ba323fcc407a89d83f2aec0dfb171/.github/actions/pull-caches/action.yml#L33

Cool, thanks for the advice. I've setup the docker and will try to figure out what's going on there.

@AndresAlgaba
Copy link
Contributor Author

@Borda @justusschock Do you have any clue why our tests failing with some old configurations? thinking

I think I remember this one, you are passing somewhere none instead of expected int:

TypeError: '<' not supported between instances of 'int' and 'NoneType'

I would suggest recreating this env and debugging locally... to do you you can use:
https://github.com/Lightning-AI/metrics/blob/50388cfb167ba323fcc407a89d83f2aec0dfb171/.github/actions/pull-caches/action.yml#L33

Cool, thanks for the advice. I've setup the docker and will try to figure out what's going on there.

Hey @stancld, doing the same currently :D Let me know if I can be of any help! It indeed seems to be some interplay with the fairlearn library.

@stancld
Copy link
Contributor

stancld commented Mar 2, 2023

@Bordafairlearn declares supports for python 3.8 and newer. What about skipping tests for python==3.7 then? It fails within the library regardless of pandas/numpy version installed with python 3.7.

setuptools.setup(
    name=fairlearn.__name__,
    version=fairlearn.__version__,
    author="Miroslav Dudik, Richard Edgar, Adrin Jalali, Roman Lutz, Michael Madaio, Hilde Weerts",
    author_email="fairlearn-internal@python.org",
    description="A Python package to assess and improve fairness of machine learning models.",
    long_description=long_description,
    long_description_content_type="text/markdown",
    url="https://github.com/fairlearn/fairlearn",
    packages=setuptools.find_packages(https://github.com/fairlearn/fairlearn/commit/4f8dddad9fe24a914db4ffd3a2f699e3248c46c5,
    python_requires=">=3.8",
    install_requires=install_requires,
    extras_require=extras_require,
    classifiers=[
        "Programming Language :: Python :: 3.8",
        "Programming Language :: Python :: 3.9",
        "Programming Language :: Python :: 3.10",
        "License :: OSI Approved :: MIT License",
        "Operating System :: OS Independent",
        "Development Status :: 3 - Alpha",
    ],
    include_package_data=True,
    zip_safe=False,
https://github.com/fairlearn/fairlearn/commit/1cbf29a45da00c5eb0ee68c5cc9e53314d229821

cc: @AndresAlgaba

(Btw, security support for python 3.7 ends in 3 months, so we can try to replace 3.7 support with newer 3.11 in general in the future.)

@mergify mergify bot added the ready label Mar 3, 2023
@stancld
Copy link
Contributor

stancld commented Mar 3, 2023

Hi @AndresAlgaba, would you please check why make-docs job is failing? 🙏 Otherwise, everything looks good now :]

@stancld
Copy link
Contributor

stancld commented Mar 3, 2023

@AndresAlgaba -- Sorry, actually found out one link for InfoLM seems to be broken.

@Borda Borda enabled auto-merge (squash) March 3, 2023 21:55
@Borda Borda merged commit 7c885d0 into Lightning-AI:master Mar 4, 2023
@Borda
Copy link
Member

Borda commented Apr 18, 2023

@AndresAlgaba thank you, and apology that it took so long...

@AndresAlgaba
Copy link
Contributor Author

@Borda, my pleasure! And thanks to the entire team for all the help :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants