Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Use EvoTrees instead of XGBoost in documentation #57
Use EvoTrees instead of XGBoost in documentation #57
Changes from 3 commits
19a09b9
faad5a9
8db3619
6abeb70
e13968c
0636445
4bac113
dac6e9b
caccd36
b352694
82dae88
07f1552
10f412f
e1d6abc
c69e32a
52e5a5a
df4772b
1cfaebc
36763fb
86baaaf
1018637
c1ca14b
80a8c68
a1f428b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I don't think we can assume that
reformat
will produce any particular output type forytest
, so perhaps better to directly index it: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.
That would be inconsistent with the suggestion above though? In line with the implementation above we could use
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.
The problem is that
xdata
andydata
are now some type that we don't know, because thereformat
output could be anything. So herextest
andytest
are correctly a subset of the rows, but we don't know thatytest
is a vector anymore. e.g. it could be aNamedTuple
with the vector stored in some field.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.
I assumed the output of
predict
would be of the same type as the reformatted labels. And in that case_rstar
would fail anyway. But maybe that assumption is not correct.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.
The docs are somewhat ambiguous on this. They say this:
However, this docs page often makes statements about the inputs
X
andy
that it clarifies are not applicable if one implementsreformat
. Since there is nounreformat
method that converts the output ofreformat
to the vector type the user is expected to provide, I think we can surmise thatpredict
is expected to return anAbstractVector
type for the predictions of the same type that the user should provide tofit
. So passingpredictions
andycategorical[test_ids]
to_rstar
should be fine.I'm not 100% sure about this, but I don't know what else might make sense. To be 100% sure about this, we would probably need to check what
predict
does for aMachine
input.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.
This is where
predict
is defined for aMachine
: https://github.com/JuliaAI/MLJBase.jl/blob/c5d755e157c853850d5f2c4b9693ddf8d9bd469a/src/operations.jl#L130-L139. There's no processing of the output ofpredict
, so it must conform to the type expected by a user (vector with the same scitype as the user-providedy
).One of the MLJ devs on Slack confirmed that outputs of
predict
are not formatted.