fauvernierma / survPen

hazard and excess hazard modelling with multidimensional penalized splines
GNU General Public License v3.0
8 stars 2 forks source link

Performance claims #10

Closed seabbs closed 5 years ago

seabbs commented 5 years ago

As part of this review openjournals/joss-reviews#1434

As mentioned in #9 I would like to see a comparison of survPen to other software. How much faster/ more stable is it. In what circumstances is it better (or is it ever worse)? When does it agree/disagree

Some of these may be in the methods paper that you are submitting but without a preprint (is there one) I can't assess this.

A good target would be to expand on this "The package mgcv can then be used to fit penalized hazard models (Remontet et al. 2018). The problem with this option is that the setup is rather complex and the method cannot be used on very large datasets for computational reasons." from the vignette with an example.

It would also be great to see survPen compared to other non-spline based methods (and get to see the big improvements it gives!).

fauvernierma commented 5 years ago

There is indeed an extensive comparison to other existing packages in the submitted paper so I have to see which elements I am allowed to make available online (for copyright reasons)

seabbs commented 5 years ago

Have you considered preprinting?

fauvernierma commented 5 years ago

Hi @seabbs,

I just added a compar folder that illustrates a comparison between survPen, mgcv and rstpm2 (another reference package in the area)

I hope this will answer your question about perfomance claim (tell me if that is not the case)

Also, I am not sure that adding this folder is the best way to go, maybe I should add this to the vignette ?

Thank you :)

seabbs commented 5 years ago

Thanks for adding this - it goes a long way to addressing my concerns. 😄

I think this would be great as a vignette - new users will probably want this information. Having it in the package documentation is also a good idea as it will likely change over time as everything gets updated.

If possible a comparison with mgcv using some of the new approaches would be good. Can you use bam (and its performance features) for your models?

fauvernierma commented 5 years ago

As you noted, the fact that the method article is not accepted yet is a bit embarrassing. In this article, survPen is validated by simulation and is compared to other existing approaches, demonstrating its superiority regarding statistical performances and/or computational time performance.

Therefore, the way I see it, survPen’s guarantee is that article (and hopefully people will soon be able to refer to it). Since the article is not yet available, I understand that you needed to make sure that the package actually did what it claimed to do. However, I do not feel comfortable referring to other existing approaches to defend survPen.

Indeed, mgcv and rstpm2 are the only other options to fit multidimensional penalized hazard and excess hazard models. However, rstpm2 can lead to severe instability (cf the comparison I provided) and even negative hazard estimation (so I am really not comfortable with referring to it). As for mgcv, no matter how well it is programmed (and we both know it is state of the art in that matter), it is not designed to fit hazard models, let alone excess hazard models (the only exception being the new coxph family but this approach does not allow modelling the effect of time). Survival models with mgcv are only possible because of a useful old trick that allows using a Poisson model to fit a survival model; this require to split the data into very small interval and corresponds to a ‘point-milieu’ approximation of the cumulative hazard (numerical approximation of the integral).

However, this tricks comes at great costs: • Using the lexis function to split the data • Using a user-defined family (instead of Poisson) in order to deal with expected mortality rates when we want to fit excess hazard models • This user-defined family has to be changed when the fitting algorithm is changed. For example, the family we used to use with the glm function had to be modified in order to fit penalized excess hazard models using gam.

That being said, I can answer your bam question :) Bam can be used to fit hazard models and I have to admit that its speed and stability are amazing. It comes toe to toe with survPen in the provided example and can be even faster with large datasets under some circumstances. However, the user-defined family used to fit excess hazard models with gam is no longer supported by bam. Maybe there is a way to make it work but if it exists it must be pretty complicated to find. Besides, even if it were possible, that would not change the fact that the data has to be split before fitting. In the provided example, this is ok because the maximum follow-up time is 5 years and the split bands are: bands=c(seq(0,1,by=0.05),seq(1.1,5,by=0.1)). As you can see, the split depends on the maximum follow-up time, and the split data can then easily get enormous when the maximum time is 20 or even 50 years (this is the case when you study the time-to-death for some chronic diseases like multiple sclerosis for example). In contrast, survPen is not affected by this. Thus, by nature the split+Poisson approach will always be more complicated to set up and more inefficient than a direct approach as the one proposed by survPen (by the way, the improvements proposed by Simon Wood with the bam function could be implemented in survPen in a future release.)

I know this is probably not the answer you were expecting and it is probably way too long but I hope you understand my concerns about having to refer to other approaches in order to promote survPen.

Thank you

seabbs commented 5 years ago

Thanks for all of this useful information! The poisson model trick was not something I had come across.

It is potentially problematic that the methods paper is not available as without it makes the software difficult to both review and use. When accepted will the paper be open or closed access? If closed source then I can imagine this bring further problems (your users will potentially not be able to confirm your software actually works etc. if not academics with access)

I'm not really sure I do understand your concerns I am afraid. The way I see it the logical thing to do would be to have a section in your current vignette, or (my preferred option) in another vignette, that details what was previously available (i.e mgcv etc.) and then walks through an example comparing these approaches to survPen. This could be very similar to your current script in compar.

If you think of an outsider looking at your work they need to be able to easily see:

I don't think you can answer these questions by just linking to an unavailable methods paper, especially if that paper won't be open access in the future.

Please correct me if I am misunderstanding something here! 😄

fauvernierma commented 5 years ago

Hello to you both,

So unless I missed something, this issue about Performance Claims should be the only one left ?

As I said in another relative issue, the details to reproduce the code from my method article will be available on the Journal website. It will normally be on this page : https://rss.onlinelibrary.wiley.com/hub/journal/14679876/series-c-datasets

and then in "Volume 68 (2019), part 5" (not yet available)

seabbs commented 5 years ago

Thanks @fauvernierma

Waiting on what @corybrunson thinks about the documentation and test issues. As I said in those issues I would like a quick start in the README, a package website and more honest tests (i.e not testing for errors/not only testing for errors).

Will the paper be open access when it goes up. To me, it looked like my institution was providing access? I think that JOSS guidelines will need the performance claims to be substantiated by open access information.

fauvernierma commented 5 years ago

The paper will not be open access but I think that the code will be (I can't access all articles on the website but I had no problem downloading code so far). Therefore with enough information on github in the README file this should be easy for people to reproduce the comparison with the other packages (as well as the simulation study from the article)

corybrunson commented 5 years ago

I believe that open-access code demonstrating the performance claims will satisfy this criterion! I don't want to check it off preemptively, though, so shall we revisit this issue when the article is published?

fauvernierma commented 5 years ago

Hi,

Indeed, I guess it is simpler if we wait for the article to be published

fauvernierma commented 5 years ago

Hello,

The article has published in early view. It is available with its Supplementary material here https://rss.onlinelibrary.wiley.com/doi/full/10.1111/rssc.12368

Normally you can access it with a University access

The associated code is available here https://github.com/fauvernierma/code_Fauvernier_JRSSC_2019

Have a great day

corybrunson commented 5 years ago

Congrats @fauvernierma ! I'll have a look at the paper and try out the comparison code ASAP.

By the way, if there are any references to this "forthcoming" paper in the survPen documentation, have you updated them?

fauvernierma commented 5 years ago

Thanks !

There is a reference at the end of the README. I will update it to inform people that they can try out the comparison code

By the way, starting from tomorrow I will be on vacation until August 19 so I will be able to see your messages but I will not always be able to update the github page

corybrunson commented 5 years ago

I'm not versed in the comparison packages but the demonstration script 0000_Compar_packages.R clearly suggests that survPen scales (in terms of combinatorial tensor effects) better than the comparator implementations. It would be nice to have the comparisons looped, so that they could be more easily and compactly expanded (e.g. to include different data sets or additional combinations of parameter settings) but that's more a suggestion for future work than an expectation for acceptance to JOSS. I think i'm satisfied.

This is @seabbs 's issue, so i'll leave it to them to make the determination. : )

seabbs commented 5 years ago

Congrats on the paper @fauvernierma! I agree that the comparison scripts do the job but also agree with @corybrunson suggestions for improvements.

I'd like to see these comparisons more clearly linked in the documentation. Ideally, as I have commented previously, these would be in the documentation but I am happy to leave that for a future update. So I think the only work left to do for JOSS acceptance is some clear links in at least the README and also potentially the vignette and paper.

fauvernierma commented 5 years ago

Hello to you both,

I just added the necessary information in the README bd70d4178efc46e64e4eb0f056909adeb294d9c1

Please let me know if anything is missing

Thank you

seabbs commented 5 years ago

Thanks - could you make it a little more obvious?

Perhaps with its own section/ higher up.

fauvernierma commented 5 years ago

Of course,

this should be better cf7ab3d4b97a144d37f0710568ce7509205b3539

seabbs commented 5 years ago

Excellent!