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

Add roc_auc_survival() #390

Merged
merged 12 commits into from
Mar 17, 2023
Merged

Add roc_auc_survival() #390

merged 12 commits into from
Mar 17, 2023

Conversation

EmilHvitfeldt
Copy link
Member

This PR is based on #378, to supply the roc_auc_survival() metric.

Copy link
Contributor

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

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

A bit hesitant to Approve as I'm not sure I have the expertise to give this a proper thumbs up, but everything looks fine to me!

R/surv-roc_survival_auc.R Outdated Show resolved Hide resolved
R/surv-roc_survival_auc.R Show resolved Hide resolved
sensitivity <- sensitivity[not_na]
specificity <- specificity[not_na]

auc( 1 - specificity, sensitivity)
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels worth noting that a different pattern exists here:

yardstick/R/prob-roc_auc.R

Lines 212 to 219 in 4a620f3

sensitivity <- curve$sensitivity
specificity <- curve$specificity
auc(
x = specificity,
y = sensitivity,
na_rm = FALSE
)

This is because direction = "minimize"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should have been "maximize" 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still a bit confused here, then—if we want to maximize this metric, shouldn't we pass specificity rather than 1 - specificity, as in the code linked above? Or is the thing labeled specificity actually one_minus_specificity in the first place?

@EmilHvitfeldt EmilHvitfeldt merged commit df058e6 into main Mar 17, 2023
@EmilHvitfeldt EmilHvitfeldt deleted the survival_curve_auc branch March 17, 2023 03:48
@EmilHvitfeldt EmilHvitfeldt changed the title Add roc_auc_survival() Add roc_auc_survival() Mar 18, 2023
@github-actions
Copy link

github-actions bot commented Apr 2, 2023

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants