Skip to content

Conversation

IyarLin
Copy link
Contributor

@IyarLin IyarLin commented Aug 25, 2018

Reference is a kaplan-meier curve model used to benchmark model performance against (since it uses no covariates). Matrix is the model performance measure. By adding "reference = F" in the pec call, I've removed the reference calculation thus speeding the code and fixing the problem of using reference instead of the fitted model error term.

R/measures.R Outdated
@@ -1429,10 +1429,8 @@ ibrier = makeMeasure(id = "ibrier", minimize = TRUE, best = 0, worst = 1,

probs = predictSurvProb(getLearnerModel(model, more.unwrap = TRUE), newdata = newdata, times = grid)
perror = pec(probs, f, data = newdata[, tn], times = grid, exact = FALSE, exactness = 99L,
maxtime = max.time, verbose = FALSE)
maxtime = max.time, verbose = FALSE, reference = F)
Copy link
Member

Choose a reason for hiding this comment

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

Please use FALSE instead of F.

R/measures.R Outdated

# FIXME: what is the difference between reference and matrix?
# FIXME: this might be the wrong number!
Copy link
Member

Choose a reason for hiding this comment

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

What about the FIXMEs you deleted? I guess that first one is irrelevant now, but what about the second?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I thought the second FIXME refers to a possible wrong number caused by the first FIXME. If that's not the case than I'm not sure what is meaning of the second FIXME. I'll put it back

@larskotthoff
Copy link
Member

Thanks, merging.

@larskotthoff larskotthoff merged commit e9e80df into mlr-org:master Aug 28, 2018
@IyarLin IyarLin deleted the fix-ibrier branch August 28, 2018 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants