Improve organization of metrics module

[hildeweerts]

Describe the issue

I think the organization of files in the fairlearn.metrics module could be improved,

both in API reference and the repository itself. My main issue regarding the documentation is that the distinction between performance metrics (e.g. recall) and fairness metrics (e.g. demographic parity) is not clear in the API reference. Additionally,

navigating the repository was not very intuitive for me. For example, some of the performance metrics not directly available in scikit-learn have their own file (e.g. _balanced_root_mean_squared_error.py), whereas others are grouped in (e.g. _extra_metrics.py).

Suggest a potential alternative/fix

I think merging and/or renaming some of the seperate files could help to improve. E.g. something like this:

• _fairness.py
  • _disparities.py
• _base_metrics.py ( _performance.py (or perhaps _classification.py and _regression.py to mimic scikit-learn)
  • _extra_metrics.py
  • _balanced_root_mean_squared_error.py
  • _mean_predictions.py
  • _selection_rate.py
• _input_manipulations.py might be better off in a utils folder in the main repository.
• I am not sure what the best place would be for _group_metric_set.py.

PS. I suppose giving each function their own page as in the scikit-learn API reference is already implicit in the documentation proposal, right?

EDIT: documentation issues moved to issue #720

[riedgar-ms]

Thanks for the feedback. I'm definitely not averse to rethinking the file structure. And _group_metric_set.py certainly needs a proper home - right now, it exists solely for the AzureML integration work I've been doing, although it could also be used in Fairlearn for a 'save this dashboard' button.

[MiroDudik]

For documentation in both API and User Guide, I think we should start with the Metrics API proposal
(which has all the concepts at one place) and add full mathematical definitions etc. to it.

Re. structuring of the modules, I totally agree that we should combine all the little files that you mention! I was thinking using the name _base_metrics instead of _performance (since they all can be either viewed as performance metrics or used to derive various fairness metrics). But I don't have a strong opinion on this.

Re. content of the package: also please take a look at the metrics that exist but are not currently surfaced in the API documentation: https://github.com/fairlearn/fairlearn/blob/master/test/unit/metrics/test_metrics_engine_dicts.py
The biggest immediate issue is to document these properly in the API docs / user guide. I'd love to be able to generate a "table" of metrics for the documentation automatically from these dictionaries...

[adrinjalali]

PS. I suppose giving each function their own page as in the scikit-learn API reference is already implicit in the documentation proposal, right?

I think that's more of a sphinx generated thing and how they're generated.

I like merging those files, @romanlutz may need to get used to it :P

I'd love to be able to generate a "table" of metrics for the documentation automatically from these dictionaries...

You could probably do that through extending or customizing sphinx, but I'd recommend having the table in a user guide nicely put in context rather than an automated table.

[hildeweerts]

I think that's more of a sphinx generated thing and how they're generated.

Yes, definitely! This was more of a quick question so I did not have to open yet another issue :P

Re. structuring of the modules, I totally agree that we should combine all the little files that you mention! I was thinking using the name _base_metrics instead of _performance (since they all can be either viewed as performance metrics or used to derive various fairness metrics). But I don't have a strong opinion on this.

Some metrics, e.g. selection_rate, are not typical evaluation metrics anyways, so _base_metrics.py works even better I think!

[riedgar-ms]

I agree that a handwritten table is likely to look much nicer, but.... if we went that route, we should give some thought to how to make sure that the table evolves with the auto-generated function list.

[romanlutz]

@adrinjalali I just want to make sure I understand: You mean having single files in the documentation is better than more nesting? I'm totally on board with that :-) Perhaps you've noticed already, but I've moved away from very fine granular documents since that means more clicking.

How about I take some time to get the table into the user guide as a PR and then we see what changes people want? I'm open to pretty much anything. The metrics were a major point of confusion for me at some point, so I'd rather avoid new users having the same experience, and that will only be possible with better documentation. Keep the feedback coming! :-)

[adrinjalali]

we should give some thought to how to make sure that the table evolves with the auto-generated function list.

Adding a new method/function is not something which happens too frequently in a library, and when it does, we should make sure we put the effort to properly document it, and updating the relevant user guide is a part of that documentation process, so I guess it should come naturally.

I just want to make sure I understand: You mean having single files in the documentation is better than more nesting?

I was talking about the .py files and not the documentation. The documentation is okay as far as I'm concerned, and the layout can always change for the better.

[romanlutz]

Looking at the user guide we actually do have a table with the performance metrics now: https://fairlearn.org/v0.6.0/user_guide/assessment.html#metrics

For this particular issue it's probably best if we focus on the organization of the metrics module (code) rather than conflating it with the documentation. #458 touches on documentation a little bit as well, but we may want to start a completely fresh issue with very specific things that should be addressed (and small in scope).

Other than that, it seems like there's agreement on merging files, so this strikes me as an issue that can move forward if there's anyone willing to take up the task.

[romanlutz]

From what I can tell the only things left here beyond what @alliesaizan did in #1174 are:

• merging metrics files as described by @hildeweerts in the initial post (I just created PR [ENH] merge metric files into _base_metrics.py, rename _disparities.py to fairness_metrics.py, and move _input_manipulations.py to the utils module #1202 for this)
• removing group metric set (done by @MiroDudik in MNT remove _group_metric_set and affiliate baggage #1187
• The metric table which already exists at https://fairlearn.org/main/user_guide/assessment/custom_fairness_metrics.html

@hildeweerts do you mind confirming? This is one of our older issues, so it's a particular highlight to close it out 🤣