search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: TorchSurv: A Lightweight Package for Deep Survival Analysis #7341

Open editorialbot opened 1 month ago

editorialbot commented 1 month ago

Submitting author: !--author-handle-->@melodiemonod<!--end-author-handle-- (Mélodie Monod) Repository: https://github.com/Novartis/torchsurv Branch with paper.md (empty if default branch): main Version: v0.1.2 Editor: !--editor-->@kanishkan91<!--end-editor-- Reviewers: @WeakCha, @LingfengLuo0510, @XinyiEmilyZhang Archive: Pending

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/02d7496da2b9cc34f9a6e04cabf2298d"><img src="https://joss.theoj.org/papers/02d7496da2b9cc34f9a6e04cabf2298d/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/02d7496da2b9cc34f9a6e04cabf2298d/status.svg)](https://joss.theoj.org/papers/02d7496da2b9cc34f9a6e04cabf2298d)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@WeakCha & @LingfengLuo0510, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @kanishkan91 know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @WeakCha

editorialbot commented 1 month ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 month ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.18 s (378.3 files/s, 67362.1 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          26           1224           2070           4135
Markdown                         7            266              0            681
TeX                              2             47              0            540
R                                7            221            158            520
Jupyter Notebook                 2              0           1551            238
YAML                             6             34              6            201
TOML                             1              8              0             49
Bourne Shell                     4             20              3             40
reStructuredText                 5             16             37             13
make                             1              5              8             10
JSON                             7              0              0              7
-------------------------------------------------------------------------------
SUM:                            68           1841           3833           6434
-------------------------------------------------------------------------------

Commit count by author:

    29  Peter Krusche
    18  Thibaud Coroller
    11  corolth1
     9  melodiemonod
     7  Mélodie Monod
     2  Peter Krusche (Novartis)
     1  Ikko Eltociear Ashimine
editorialbot commented 1 month ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.21105/joss.01317 is OK
- 10.32614/CRAN.package.survival is OK
- 10.48550/arXiv.1912.01703 is OK
- 10.5281/zenodo.3352342 is OK
- 10.32614/CRAN.package.survAUC is OK
- 10.1109/cvpr42600.2020.00975 is OK
- 10.1186/s12874-018-0482-1 is OK
- 10.48550/arXiv.2204.07276 is OK
- 10.1007/978-1-4612-4380-9_37 is OK
- 10.1016/s0197-2456(03)00072-2 is OK
- 10.32614/CRAN.package.survival is OK
- 10.32614/CRAN.package.survival is OK
- 10.32614/CRAN.package.survAUC is OK
- 10.32614/CRAN.package.timeROC is OK
- 10.32614/CRAN.package.risksetROC is OK
- 10.32614/CRAN.package.survivalROC is OK
- 10.1093/bioinformatics/btr511 is OK
- 10.32614/CRAN.package.riskRegression is OK
- 10.32614/CRAN.package.SurvMetrics is OK
- 10.32614/CRAN.package.pec is OK
- 10.1111/j.0006-341x.2000.00337.x is OK
- 10.1111/j.0006-341x.2005.030814.x is OK
- 10.1002/bimj.201200045 is OK
- 10.1198/016214507000000149 is OK
- 10.1093/biostatistics/kxy006 is OK
- 10.1002/sim.4154 is OK
- 10.1002/(sici)1097-0258(19960229)15:4<361::aid-sim168>3.0.co;2-4 is OK
- 10.1002/(sici)1097-0258(19990915/30)18:17/18<2529::aid-sim274>3.0.co;2-5 is OK
- 10.1080/01621459.1977.10480613 is OK
- 10.2307/1402659 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Time-to-Event Prediction with Neural Networks and ...
- No DOI given, and none found for title: The Weibull Distribution

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
editorialbot commented 1 month ago

Paper file info:

📄 Wordcount for paper.md is 1374

✅ The paper includes a Statement of need section

editorialbot commented 1 month ago

License info:

✅ License found: MIT License (Valid open source OSI approved license)

editorialbot commented 1 month ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

kanishkan91 commented 1 month ago

@melodiemonod, @WeakCha, @LingfengLuo0510, This is the review thread for the paper. All of our communications will happen here from now on.

Please read the "Reviewer instructions & questions" in the first comment above.

For @WeakCha, @LingfengLuo0510 - Both reviewers have checklists at the top of this thread (in that first comment) with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

As you are probably already aware, The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/7341 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

Thanks in advance and let me know if you have any questions!!

WeakCha commented 1 month ago

Review checklist for @WeakCha

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

editorialbot commented 1 month ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

WeakCha commented 1 month ago

@melodiemonod @kanishkan91

Thank you so much for this package, this work looks super interesting! Here are my initial comments:

For the paper:

  1. Adding an example after you introduce the functionality would be great. This example does not need to be comprehensive, but needs to be executable without any error. Your tutorial has done a great job on this, but people may refer to your paper first and want to seek for something that is easier to use.
  2. There are some typos in your paper. For example, scitkit-survival should be scikit-survival in Figure 1, MyPyTorcWeibullhModel should be MyPyTorchWeibullModel (between line 46 and 47), and MyPyTorchXCoxModel should be MyPyTorchCoxModel (between line 51 and 52).

For your tutorial (https://opensource.nibr.com/torchsurv/notebooks/introduction.html), could you please show what helpers_introduction is? I know there should be some plugins but I cannot find them, and I will give further comments after I successfully reproduce your tutorial.

Thank you so much for your work! I am looking forward to reviewing your updated paper and package!

tcoroller commented 1 month ago

Dear @WeakCha,

Thank you for your early comments. Regarding your last one (helpers_*), would it be ok with you if we add this line below in the notebooks with the link to the helper file itself?

For the first notebook (introduction), the helpers_introduction line will be the following: 

# PyTorch boilerplate - see https://github.com/Novartis/torchsurv/blob/main/docs/notebooks/helpers_introduction.py
from helpers_introduction import Custom_dataset, plot_losses

I would discourage adding the code directly in the notebook because it would overcrowd the notebook:

  1. I would need to be at the beginning of the notebook to be properly loaded and run, which is annoying because one helper (helpers_momentum) is ~200lines of code
  2. They are simply boilerplate PyTorch code, with no relationship to TorchSurv and well established function (model and datasets, plot losses).

If our suggestion isn't satisfying, I can look into other option (one could be to have the cell hidden, but requires another package thus making it rely on more dependancies)

tcoroller commented 1 month ago

Hello everyone,

We modified the package according to the first comments. Please use the main branch (default) for the review as we will keep this one up to date with your comments.

Thanks

WeakCha commented 3 weeks ago

Hi @tcoroller Thanks for the update! My further comments about your tutorial are listed below:

  1. I am guessing whether the learning rate 10^{-2} is too high for your coxph experiment as your loss value changes turbulently in both training and test data. For better visualization effects maybe you could try a lower learning rate.
  2. Your helper_function can not be imported after I installed torchsurv. I found your discussion and I guess the most efficient way is to give necessary instructions/comments in your notebook about how to make your helper functions work (for example, personally I will simply add the helper functions to my own notebook, although this will make the notebook more lengthy), so that readers will not be confused by this, or will not need to add the content of the helper function by themselves. I have no preference on whether you like or dislike adding helper functions explicitly in your notebook.
  3. I am running your MNIST tutorial on an environment without GPU and 12G RAM, and it crashed. Do you have comments on this? I guess a large memory is required to reproduce this tutorial, and you could add some information about it.
  4. Have you done some validity tests for your package? For example, your model results should be comparable with those in other established packages for the same dataset with the same training/test split, for the same deep learning model architecture, and for the same loss function. A notebook like this will be highly welcomed, but I would not insist on this as this request may be time consuming, and I could also spend time checking your implementations. What about your thoughts?
  5. There are some display effects that I think might not be the most suitable. For example, in your introduction tutorial, in the "Dependency" section, this sentence:

"the recommanded method is to use our developpment conda environment (preferred)."

seems to emphasize the word "preferred" with a code font. I guess you want to use bold or italic instead? There are many words like this in your tutorials.

  1. There are also some grammar errors or typos, for example, "recommanded" should be "recommended", "this notebooks" should be "this notebook".

I will get back to you with paper reviews if any. Thanks a lot for your work!

WeakCha commented 3 weeks ago

@editorialbot generate pdf

editorialbot commented 3 weeks ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

WeakCha commented 3 weeks ago

@tcoroller @melodiemonod @kanishkan91

It seems that your paper is unchanged. I guess this may be an error as I found an updated version here: https://github.com/Novartis/torchsurv/blob/58-comments-joss-241018/paper/paper.md, but I cannot get this when following the prompt by the editoralbot.

tcoroller commented 3 weeks ago

dear @WeakCha ,

Thank you for your comments, please see my replies below:

  1. Learning rate: You may be referring to the introduction notebook. Indeed, the Cox model isn't doing well, with a poor looking training loss profile. However, this notebook also shows the Weibull model doing pretty well and then ultimely compare the two to illustrate our functions, such as this key function cindex.compare(). This notebook is meant to illustrate the function rather than pushing for performance, and having Cox model performing poorly was good to this purpose. Please let me know that clarifies and give you better context.

  2. helper_functions: Sorry for the confusion there, those functions are not part of TorchSurv but only support the notebooks. Installing TorchSurv will not solve this, but those function are modules sitting directly next to the notebooks, so the import is relative and should run directly. You need to make sure you have all files together, or better clone the whole repository to be safe. Please see below in the screenshot.

    Screenshot 2024-10-31 at 9 42 24 AM

  3. MNIST: Appologies for the trouble running this notebook. I made this example on a M2 macbook with 64Gb of memory, so I guess I didn't think too much of batch size limitation. Several colleagues tried it on HPC, but I would assume with GPUs. I will add a code snippet to downgrade the batch size (currently 500) to 50 or less to make sure it runs smoothly on smaller computer if the user is using CPUs.

  4. Model comparison: Thank you for this comment, this is a very important part of our work to ensure that our package is comparable against established R/python packages. During the comparison, we focused on simple model (single neuron, batch size equal to dataset) to mimic other packages and validate the code. We did not compare our method to other deep learning package, simply because most are either deprecated (last commit >3 years ago), poorly documented or simply out of scope. We have an extensive list of comparison for all our metrics that can be found here. @melodiemonod is the lead for this piece and can comment further.

  5. Formatting: Thank you for the comment and sorry for the inconsistent formatting. We will review and edit the document accordingly.

  6. Typos: Thank you for the comment and sorry for the typos. We will review and edit the document accordingly.

  7. Paper: Thank you for raising the issue about the paper not being update. We are now working on the main branch but I think the editorialbot is still look at an older branch. @kanishkan91 could you help update the Branch with paper.md to main?

Thank you, Thibaud

melodiemonod commented 3 weeks ago

To expand on point 4 about model comparison: our package is designed to provide all the necessary functions to assist in model fitting (i.e.., with the negative log-likelihood function) and evaluate its performance (i.e., with evaluation metric functions), rather than providing functions for directly fitting a model. Unlike packages like deepsurv, we do not provide functions like fit for direct model training. Instead, our package focuses on offering users maximum flexibility, allowing them to integrate our functions seamlessly within their own PyTorch-based model-fitting workflows. As a result, comparing model-fitting outcomes with another algorithm falls outside the scope of our package. However, we have included a comparison to demonstrate that our likelihood and metric functions produce results consistent with established package. This is the notebook that Thibaud mentioned above.

WeakCha commented 3 weeks ago

@tcoroller @melodiemonod Thank you so much for your reply and I agree with most of them. I would wait to review

  1. Your MNIST example.
  2. The updated paper and tutorial (without formatting issues).

Again, thanks a lot for your work!

melodiemonod commented 2 weeks ago

@editorialbot commands

editorialbot commented 2 weeks ago

Hello @melodiemonod, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# Run checks and provide information on the repository and the paper file
@editorialbot check repository

# Check the references of the paper for missing DOIs
@editorialbot check references

# Generates the pdf paper
@editorialbot generate pdf

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers
melodiemonod commented 2 weeks ago

@editorialbot set main as branch

editorialbot commented 2 weeks ago

Done! branch is now main

melodiemonod commented 2 weeks ago

@editorialbot generate pdf

editorialbot commented 2 weeks ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

melodiemonod commented 2 weeks ago

@WeakCha Please find updated pdf in the comment above. Many thanks for your patience and for your helpful comments.

@tcoroller FYI, review branch has been changed to main.

tcoroller commented 2 weeks ago

Dear @WeakCha,

I have updated accordingly the documentation (see Pull Request) with the requested changes.

Thank you! Thibaud

melodiemonod commented 3 days ago

Dear @LingfengLuo0510 (cc @kanishkan91 )

We would greatly appreciate it if you could submit your review of the paper at your earliest convenience. Thank you in advance for your time and valuable insights. Please don’t hesitate to reach out if you have any questions or need additional information.

WeakCha commented 2 days ago

Thanks for reminding me this (as Github sends email to me every time there is an update in the thread). I am swamped recently but will make sure to spend some time reviewing your updates next week. Thank you so much for your understanding!

kanishkan91 commented 2 days ago

@tcoroller @melodiemonod - Sorry about the late reply. I just got back from vacation. I'l look for another reviewer as a safety while we wait for @LingfengLuo0510 to respond.

@LingfengLuo0510- Could you provide an update on your review?

kanishkan91 commented 2 days ago

@XinyiEmilyZhang, your name was recommended as a potential reviewer. Would you be interested in reviewing this package for the Journal of Open Source Software (JOSS)?

XinyiEmilyZhang commented 2 days ago

Thanks for letting me know! I’d like to review the package. Please let me know how to proceed.

Emily

On Wed, Nov 20, 2024 at 7:34 PM Kanishka Narayan @.***> wrote:

@XinyiEmilyZhang https://github.com/XinyiEmilyZhang, your name was recommended as a potential reviewer. Would you be interested in reviewing this package for the Journal of Open Source Software (JOSS)?

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/7341#issuecomment-2489824098, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOGSXBI2OXSDGQIV2NTGK332BUTAXAVCNFSM6AAAAABPVX3BWKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBZHAZDIMBZHA . You are receiving this because you were mentioned.Message ID: @.***>

kanishkan91 commented 2 days ago

@editorialbot add @XinyiEmilyZhang as reviewer

editorialbot commented 2 days ago

@XinyiEmilyZhang added to the reviewers list!

kanishkan91 commented 2 days ago

@XinyiEmilyZhang I'be added you as a reviewer. You should get an email regarding the same on your email (the ID linked to your GitHub). Adding instructions below-

Reviewers have checklists at the top of this thread (in that first comment) with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

As you are probably already aware, The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/7341 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

Thanks in advance and let me know if you have any questions!!

LingfengLuo0510 commented 2 days ago

Sorry for the delay. I will post my comments this week.

XinyiEmilyZhang commented 17 hours ago

@kanishkan91, thanks for adding me as a reviewer and providing all the necessary instructions.

I’m wondering if I should start the review now or wait for @LingfengLuo0510 to complete the first round as the review might have already started from the original reviewer side. However, I’d like to note that we’re running very short on time, as the original timeline suggests 4-6 weeks to complete the review process, though having three reviewers working simultaneously could complicate things for both the authors and editors.

Could you please advise on how best to proceed? Thanks so much.

kanishkan91 commented 17 hours ago

@XinyiEmilyZhang Kindly start your review as soon as convenient. Regarding the timeline, please note that the 4-6 weeks is a suggested ideal timeline. Reviews end up taking longer or shorter than that. I need a minimum of two reviews before proceeding on the paper, however the more the better. If you can complete your review in the next couple of weeks that would be the most ideal case.