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

ENH - Implement Cox with Efron estimate #159

Merged
merged 52 commits into from Jun 8, 2023

Conversation

Badr-MOUFAD
Copy link
Collaborator

@Badr-MOUFAD Badr-MOUFAD commented May 30, 2023

A follow-up of #157,

Handling tied data using Efron estimate can be obtained by slightly modifying the Cox datafit as follows

$$ l(\beta) = -\langle s, \mathbf{X}\beta \rangle + \langle s, \log(\mathbf{B}e^{\mathbf{X}\beta} - \mathbf{A}e^{\mathbf{X}\beta}) \rangle $$

where $\mathbf{A}$ is a matrix chosen accordingly to account for the additional term in the Breslow $\log$.
Also, evaluating $\mathbf{A} v$ or $\mathbf{A}^\top v$ is cheap and can be obtained in linear time.


Link to the maths behind

@mathurinm
Copy link
Collaborator

Maybe you can add a small section at the end of the doc example with lifelines to show that we handle ties the same way lifelines does? no need for another speed benchmark, just showing that we have this functionality too

@Badr-MOUFAD
Copy link
Collaborator Author

Maybe you can add a small section at the end of the doc example with lifelines to show that we handle ties the same way lifelines does? no need for another speed benchmark, just showing that we have this functionality too

Yes, I agree with the idea!

@mathurinm
Copy link
Collaborator

The edited example and benchmark in this PR may interest @ogrisel too :)

@mathurinm mathurinm merged commit 395af5e into scikit-learn-contrib:main Jun 8, 2023
4 checks passed
@ogrisel
Copy link

ogrisel commented Jun 8, 2023

Haha the benchmark plot! 😅

@ogrisel
Copy link

ogrisel commented Jun 9, 2023

BTW do you see a similar perf improvement for smooth penalties?

@Badr-MOUFAD
Copy link
Collaborator Author

@ogrisel we have added support for L2 regularization too (with scipy LBFGS and our Prox Newton methods).
Check the bench figure

L2-Cox-bench-figure

Also, here is the link to the complete benchmark and the repo to reproduce it.

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.

None yet

3 participants