Skip to content

Conversation

@topepo
Copy link
Member

@topepo topepo commented Oct 27, 2025

Adds engines for classification, regression, and quantile regression.

Tests in tidymodels/extratests#281

@topepo topepo marked this pull request as ready for review October 28, 2025 14:47
@topepo topepo requested a review from hfrick October 29, 2025 12:09
Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

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

Engine-specific tuning parameters are not part of this (yet?) - that's on purpose, right?

I've focused on the changes directly related to grf, the others looked mostly like air-related.



# ------------------------------------------------------------------------------
# grf components
Copy link
Member

Choose a reason for hiding this comment

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

Nit: It'd be nice if the ordering here was consistent. Either each "thing" for all modes or everything for one mode, then the next mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I understand correctly, the pattern that is in parnsip is to run all of the set_model_engine() lines for each mode then all of the set_dependency() calls for each mode.

I don't think that we are ordering anything different for grf than the others in this file or in other files.

Comment on lines 3 to 5
#' The \pkg{grf} fits models that create a large number of decision
#' trees, each independent of the others. The final prediction uses all
#' predictions from the individual trees and combines them.
Copy link
Member

Choose a reason for hiding this comment

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

This reads very general, and usually we try to be a bit more specific in this description block. Is there anything else we could say here? I do realize that this is about generalized random forests so there might not be!

Copy link
Member Author

Choose a reason for hiding this comment

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

I made a very small tweak, but I don't really think that there is anything additional to say. It's a generalization of random forests (and not different from the other details files). We do have a reference, so people can look there for more details.

The main reason we have that description block is to avoid saying "random forest" since some people may not know what it is. It's not meant to be very detailed.

@topepo topepo merged commit 282d081 into main Oct 31, 2025
14 checks passed
@topepo topepo deleted the qrf branch October 31, 2025 14:20
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.

3 participants