-
Notifications
You must be signed in to change notification settings - Fork 216
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
Faster speed for compare_survival #215
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your help!
The algorithm looks good to me, I just would use a couple more numpy functions.
sksurv/compare.py
Outdated
@@ -75,6 +75,7 @@ def compare_survival(y, group_indicator, return_stats=False): | |||
observed = numpy.zeros(n_groups, dtype=numpy.int_) | |||
expected = numpy.zeros(n_groups, dtype=numpy.float_) | |||
covar = numpy.zeros((n_groups, n_groups), dtype=numpy.float_) | |||
group_eye = numpy.eye(n_groups, dtype=bool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are only using it for indexing covar
, using numpy.diag_indices
makes more sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know such thing exist, sure, way less memory consumption
sksurv/compare.py
Outdated
covar[g1, g2] -= temp * at_risk[g2] / total_at_risk | ||
temp = at_risk * multiplier | ||
covar[group_eye] += temp | ||
covar -= (temp[:, None] * at_risk[None, :] / total_at_risk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need the brackets, and temp[:, None] * at_risk[None, :]
is the same as numpy.outer(temp, at_risk)
.
Codecov Report
@@ Coverage Diff @@
## master #215 +/- ##
==========================================
- Coverage 98.33% 98.33% -0.01%
==========================================
Files 37 37
Lines 3127 3126 -1
Branches 460 458 -2
==========================================
- Hits 3075 3074 -1
Misses 28 28
Partials 24 24
Continue to review full report at Codecov.
|
Thanks! Merged in commit ccc6913 |
Checklist
compare_survival
function #214What does this implement/fix? Explain your changes
numpy
native syntax/ operation to accelerate the python iteration on a huge cartetian productchisq
,pvalue
,covar
matrix,table