-
Notifications
You must be signed in to change notification settings - Fork 74
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
Evaluation updated to competing risks #29
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.
lets change the name of this function to _eval_nll
dsm/dsm_api.py
Outdated
if not(self.fitted): | ||
raise Exception("The model has not been fitted yet. Please fit the " + | ||
"model using the `fit` method on some training data " + | ||
"before calling `eval`.") |
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.
change eval to _eval_nll
dsm/dsm_api.py
Outdated
@@ -120,6 +121,26 @@ def fit(self, x, t, e, vsize=0.15, | |||
|
|||
return self | |||
|
|||
def _eval_nll(self, x, t, e): | |||
""" | |||
Evaluates models with conditional loss |
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.
Add more descriptive docstring: specify what x, t , e requires and what the output is. The docstring should clearly describe that this function outputs the negative log likelihood of the dsm model
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.
Also specify how NLL is calculated in the presence of multiple competing risks
dsm/dsm_api.py
Outdated
@@ -32,7 +32,8 @@ | |||
from dsm.dsm_torch import DeepConvolutionalSurvivalMachinesTorch | |||
from dsm.losses import predict_cdf | |||
import dsm.losses as losses | |||
from dsm.utilities import train_dsm, _get_padded_features, _get_padded_targets | |||
from dsm.utilities import train_dsm, _get_padded_features, _get_padded_targets, \ |
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.
break into 3 separate import statements 1) train_dsm, 2) _get_padded_features, _get_padded_targets and 3) _reshape_tensor_with_nans
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.
@Jeanselme would this work for both simple DSM and Recurrent DSM ? Please check, ideally it should. Also look at the inline comments I have added.
Yes, it does |
Update followings comments
LGTM. Merged. |
* Implemented computation of Negative Log-Likelihood.
* conv_model * Update .travis.yml * modified: dsm/dsm_torch.py * modified: dsm/dsm_api.py modified: dsm/dsm_torch.py * fixed_syntax_small_bugs * removed_data * Change fit return to match Sklearn format * linting * torch * linting * whitespace * linting * removed_trailing_whitespace * modified: dsm/dsm_api.py modified: dsm/dsm_torch.py modified: examples/conv_example.ipynb modified: dsm/utilities.py modified: examples/conv_example.ipynb modified: examples/pbc_final_experiment.ipynb * Fix validation loss computation * modified: docs/dsm_api.html modified: docs/dsm_torch.html modified: docs/losses.html * Evaluation updated to competing risks (autonlab#29) * Implemented computation of Negative Log-Likelihood. * modified: docs/dsm_api.html modified: dsm/dsm_api.py * modified: requirements.txt * modified: requirements.txt * Update README.md * modified: docs/datasets.html modified: docs/dsm_api.html modified: docs/dsm_torch.html modified: docs/index.html modified: docs/losses.html modified: docs/utilities.html * Change multi risk for irregular length (autonlab#32) * Update README.md * Update README.md * Update README.md Co-authored-by: Chufan Gao <[email protected]> Co-authored-by: Chirag Nagpal <[email protected]> Co-authored-by: Chirag Nagpal <[email protected]> Co-authored-by: Vincent Jeanselme <[email protected]>
* conv_model * Update .travis.yml * modified: dsm/dsm_torch.py * modified: dsm/dsm_api.py modified: dsm/dsm_torch.py * fixed_syntax_small_bugs * removed_data * Change fit return to match Sklearn format * linting * torch * linting * whitespace * linting * removed_trailing_whitespace * modified: dsm/dsm_api.py modified: dsm/dsm_torch.py modified: examples/conv_example.ipynb modified: dsm/utilities.py modified: examples/conv_example.ipynb modified: examples/pbc_final_experiment.ipynb * Fix validation loss computation * modified: docs/dsm_api.html modified: docs/dsm_torch.html modified: docs/losses.html * Evaluation updated to competing risks (autonlab#29) * Implemented computation of Negative Log-Likelihood. * modified: docs/dsm_api.html modified: dsm/dsm_api.py * modified: requirements.txt * modified: requirements.txt * Update README.md * modified: docs/datasets.html modified: docs/dsm_api.html modified: docs/dsm_torch.html modified: docs/index.html modified: docs/losses.html modified: docs/utilities.html * Change multi risk for irregular length (autonlab#32) * Update README.md * Update README.md * Update README.md Co-authored-by: Chufan Gao <[email protected]> Co-authored-by: Chirag Nagpal <[email protected]> Co-authored-by: Chirag Nagpal <[email protected]> Co-authored-by: Vincent Jeanselme <[email protected]>
No description provided.