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.

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

fitted.mboost() breaks when called with argument newdata #76

Closed
sbrockhaus opened this issue May 4, 2017 · 5 comments
Closed

fitted.mboost() breaks when called with argument newdata #76

sbrockhaus opened this issue May 4, 2017 · 5 comments

Comments

@sbrockhaus
Copy link
Member

@sbrockhaus sbrockhaus commented May 4, 2017

I am not sure whether this behaviour is on purpose or not. Anyway, fitted.mboost() breaks when called with the argument newdata.

library(mboost)
data("bodyfat", package = "TH.data")
mod <- mboost(DEXfat ~ btree(age) + bols(waistcirc) + bbs(hipcirc),
                data = bodyfat)

fitted(mod, newdata = bodyfat)

yields

Error in predict.mboost(object, newdata = NULL, ...) : 
  formal argument "newdata" matched by multiple actual arguments

It would be very easy to fix fitted.mboost() such that it behaves just as predict.mboost() when called with newdata by deleting newdata=NULL in https://github.com/boost-R/mboost/blob/master/R/methods.R#L211:

### compute fitted values
fitted.mboost <- function(object, ...) {
  args <- list(...)
  if (length(args) == 0) {
    ret <- object$fitted()
    if (length(ret) == length(object$rownames))
      names(ret) <- object$rownames
  } else {
    ret <- predict(object, ...)
    #if (NROW(ret) == length(ret))
    #    rownames(ret) <- object$rownames
  }
  ret
}

The question is, do we want this behaviour? Or generally ignore newdata in fitted()? Or throw a warning / error?

@hofnerb
Copy link
Member

@hofnerb hofnerb commented May 4, 2017

I see that it would be easy to fix the issue but I would hesitate to do that as fitted() should extract fitted values and not make predictions. That is what predict is for. Hence, I think we should discard newdata and issue a warning that this has happened. Can you provide a fix or shall I go ahead?

@hofnerb
Copy link
Member

@hofnerb hofnerb commented May 4, 2017

Btw. I am planning to submit mboost today (given Torstens patch works) and we should hence submit gamboostLSS tomorrow (and then FDboost).

@sbrockhaus
Copy link
Member Author

@sbrockhaus sbrockhaus commented May 4, 2017

To be honest, I would be happy if you could provide a fix for newdata. I am just into testing...
Thanks for the update. I will finish testing tonight :-)

@hofnerb
Copy link
Member

@hofnerb hofnerb commented May 4, 2017

Fine. I am on my way :)

@sbrockhaus
Copy link
Member Author

@sbrockhaus sbrockhaus commented May 4, 2017

Thanks a lot!

@hofnerb hofnerb closed this in e0fa2eb May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.