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.

Sign up for GitHub

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

Jump to bottom

Time varying prototype #82

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

Time varying prototype #82

wants to merge 21 commits into from
+1,323 −19

Conversation

tcoroller
Copy link
Collaborator

first draft

@tcoroller tcoroller added the enhancement ✨ New feature or request label Dec 11, 2024
@tcoroller tcoroller added this to the 0.1.4 milestone Dec 11, 2024
@tcoroller tcoroller linked an issue Dec 11, 2024 that may be closed by this pull request
Time-Varying Covariates #70
Open
@tcoroller tcoroller marked this pull request as draft December 11, 2024 19:54
@tcoroller tcoroller modified the milestones: 0.1.4, backlog Dec 13, 2024
@tcoroller tcoroller modified the milestones: backlog, 0.1.5 Jan 3, 2025
@tcoroller tcoroller marked this pull request as ready for review January 3, 2025 15:03
@tcoroller tcoroller self-assigned this Jan 3, 2025
SoniaDem
SoniaDem approved these changes Jan 3, 2025
View reviewed changes
Copy link
Collaborator

@SoniaDem SoniaDem left a comment

Choose a reason for hiding this comment

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

Checks make sense :)

@tcoroller
Copy link
Collaborator Author

tcoroller commented Jan 7, 2025 •
edited
Loading

@melodiemonod my part is done for now regarding the source code file. Can you check the code and examples?

@SoniaDem:

  • Will be checking the loss value validation (without training) to benchmark our version (loss_torchsurv vs others)
  • Check the time varying covariate
  • Ping @tcoroller for review
  • then I ping @melodiemonod for last round of review

@tcoroller tcoroller marked this pull request as draft January 7, 2025 17:07
@melodiemonod
Copy link
Collaborator

Hi @tcoroller, Do I need to review this now?

@melodiemonod
Copy link
Collaborator

@tcoroller

Notebook introduction
1/ The validation error is increasing in the notebook, I'm not sure that's a good look.
2/ I think box [30] could be removed.
3/ I'm confused if there have been changes linked to time-varying covariates there?

@tcoroller
Copy link
Collaborator Author

tcoroller commented Jan 13, 2025 •
edited
Loading

@tcoroller

Notebook introduction 1/ The validation error is increasing in the notebook, I'm not sure that's a good look. 2/ I think box [30] could be removed. 3/ I'm confused if there have been changes linked to time-varying covariates there?

@melodiemonod for now only the python file should be within scope, the notebook is still a WIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SoniaDem SoniaDem SoniaDem approved these changes

@melodiemonod melodiemonod Awaiting requested review from melodiemonod

Assignees

@tcoroller tcoroller

@SoniaDem SoniaDem

Labels
enhancement ✨ New feature or request
Projects
None yet
Milestone
0.1.5
Development

Successfully merging this pull request may close these issues.

Time-Varying Covariates
3 participants
@tcoroller @melodiemonod @SoniaDem