ENH Plot ROC AUC curves #869
Conversation
| Reference | ||
| ---------- | ||
| MetricFrame: | ||
| https://fairlearn.org/v0.6.2/api_reference/fairlearn.metrics.html |
There was a problem hiding this comment.
This should be a reference to the class in the docs, not a full URL
There was a problem hiding this comment.
Yep. Will fix when I run the documentation (I didn't run Sphinx yet.)
| for name in sensitive_index: | ||
| grp_y_true = self.sensitive_series[name][0] | ||
| grp_y_score = self.sensitive_series[name][1] | ||
| display = self.plot_roc( |
There was a problem hiding this comment.
Is it possible to use self.plot_roc as the metric itself, with judicious use of functools.partial to bind the other arguments (not sure if that works with **kwargs).
There was a problem hiding this comment.
@riedgar-ms Yeah, I tried that at first. It may be possible, but when I tried it I ran into some issues with the MetricFrame class because it computes the metric on instantiation, and all I am actually using from that class is the group by function to split the data. It was far easier to pass a placeholder function. Open to suggestions. However, I think the work that needs to happen is some small tweaks to MetricFrame..So holding out for us to determine the approach to MetricFrame and then we can come back to this and refactor it if needed.
There was a problem hiding this comment.
@kstohr thanks for sharing, this is great! I've put in a few preliminary comments. It would be nice to have a few screenshots of how it renders in the example, and a brief PR description that links to the corresponding issue. Let me know if you have questions or doubts about my comments. I'm always happy to chat!
| """ | ||
| Provides utilties for generating AUC scores, ROC curves | ||
| and plotting ROC curves grouped by sensitive feature. Depends on | ||
| the Fairlearn MetricFrame class to split the input by sensitive feature. |
There was a problem hiding this comment.
nit: use ReST directives, e.g.,
:class:`fairlearn.metrics.MetricFrame`
There was a problem hiding this comment.
@romanlutz Will that markdown automatically create a link to the API in the docs when I build the docs?
There was a problem hiding this comment.
Yes! It's restructured text (ReST) rather than markdown, but that's exactly what it does. I go back to the ReST guide quite a lot these days: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html
|
Ok, let me make those changes. We still have to build tests and I didn't actually run sphinx. I wrote the example, but didn't really look at how it will format in the docs. I also would like to give some direction on how you would mitigate a model once having run the ROC Curves. |
|
@kstohr thanks for patiently working through the comments 🙂 In terms of mitigation I would suggest restricting this PR to generating the ROC curves. It will keep growing in scope otherwise and there may be different viewpoints on how one can mitigate that, which will drag everything out quite a bit. It's probably preferable to keep the change as small as possible and then add to it in follow-up PRs. We can certainly get the conversation started, though! |
|
@kstohr do you want to revisit this and perhaps set up some time to talk about testing as we had planned a while back? |
Ping @kstohr 🙂 I suppose we can go ahead an assume that others can take over this PR if we don't hear back by 12/20 (two weeks from now). If you're still interested in getting back to this at a later point, @kstohr, feel free to say so! |
|
Sure thing. Let’s find a time.
…On Mon, Dec 6, 2021 at 7:17 PM Roman Lutz ***@***.***> wrote:
@kstohr <https://github.com/kstohr> do you want to revisit this and
perhaps set up some time to talk about testing as we had planned a while
back?
Ping @kstohr <https://github.com/kstohr> 🙂 I suppose we can go ahead an
assume that others can take over this PR if we don't hear back by 12/20
(two weeks from now). If you're still interested in getting back to this at
a later point, @kstohr <https://github.com/kstohr>, feel free to say so!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#869 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB6WM55PD2PIXAUEVLTUFSLUPV4GLANCNFSM4642IFKQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
…_score to data_for_test,updates _roc_auc to comply with updated MetricFrame class instantiation
No description provided.