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

response(fitted()) and fitted(, type = response) in mboostLSS.R #5

Closed
sbrockhaus opened this issue Oct 26, 2015 · 2 comments
Closed

response(fitted()) and fitted(, type = response) in mboostLSS.R #5

sbrockhaus opened this issue Oct 26, 2015 · 2 comments

Comments

@sbrockhaus
Copy link
Member

@sbrockhaus sbrockhaus commented Oct 26, 2015

This is just a question of understandig some of your code:

In the file mboostLSS.R it says in Code line 159ff:

## update value of nuisance parameters
## use response(fitted()) as this is much quicker than fitted(, type = response)
 for (k in mods[-j])
     assign(names(fit)[k], families[[k]]@response(fitted(fit[[k]])),
                environment(get("ngradient", environment(fit[[j]]$subset))))

And consequently response(fitted()) is used.

I just wondered whether there is a reason, that in line 130 fitted(, type = response) is used instead of response(fitted())?
The corresponding code ist:

for (k in mods[-j]){
    if (!is.null(fit[[k]]))
          assign(names(fit)[k], fitted(fit[[k]], type = "response"),
               environment(families[[j]]@ngradient))
}

Best

@hofnerb
Copy link
Member

@hofnerb hofnerb commented Oct 29, 2015

Dear Sarah,

very good question.

The reason for changing fitted(, type = response) to response(fitted()) in lines 159ff was that fitted(, type = response) actually calls predict(, type = response) and thus needs to do all computations anew while fitted() simply returns the current accumulated fit.

If we predict in line 130, this is not such a problem as we only have offset models so far. In 159ff we have models with potentially many boosting steps (depending on the current iteration).

Having said this, I think it is not really necessary to change line 130 but it doesn't hurt either. For consistency, we thus modified the code.

@hofnerb hofnerb closed this in 7915e60 Oct 30, 2015
@sbrockhaus
Copy link
Member Author

@sbrockhaus sbrockhaus commented Nov 2, 2015

Thanks a lot for the explanations!

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.