Closed jmarshallnz closed 2 years ago
I may be misunderstanding, but can you clarify what you are wanting to be able to do that you can't do right now? You can set these as engine arguments right now with the current CRAN version of discrim (another example here):
library(discrim)
#> Loading required package: parsnip
discrim_linear() %>%
fit(Species ~ ., data=iris) %>%
purrr::pluck("fit", "prior")
#> setosa versicolor virginica
#> 0.3333333 0.3333333 0.3333333
discrim_linear() %>%
set_engine("MASS", prior=c(2,1,2)/5) %>%
fit(Species ~ ., data=iris) %>%
purrr::pluck("fit", "prior")
#> setosa versicolor virginica
#> 0.4 0.2 0.4
Created on 2022-03-21 by the reprex package (v2.0.1)
Can you be a bit more specific in what you can't do right now that you want to be able to do?
Aha! This is my complete misunderstanding. Will close.
I assumed that arguments in set_engine()
that weren't listed in defaults
were not available to the user. This is clearly not the case, so I'll try and explain why I thought that in case it might be useful!
I think it was because:
fit()
instead of set_engine()
which obviously doesn't work.default
, i.e. those listed via translate()
.These three things were enough for me to reach the conclusion that you couldn't add other arguments. Enough that I didn't even try it! (This is clearly a bit silly, so maybe nothing needs changing?)
I do notice that the help for set_engine()
is adjusting an argument that isn't in defaults
. I missed that completely! But I wonder if it could be clarified further, along the lines of "The ...
argument of set_engine()
allows any engine-specific argument to be passed directly to the engine fitting function, e.g. ranger
has an importance
argument, which could be specified in set_engine
.
I would be happy to do up a PR for the parsnip::set_engine()
docs if you feel it would be useful.
Yeah, if you'd like to open a PR on parsnip to change this:
Engine arguments are either specific to a particular engine or used more rarely; there is no change for these argument names from the underlying engine. Set these in
set_engine()
, likeset_engine("ranger", importance = "permutation")
.
to this:
Engine arguments are either specific to a particular engine or used more rarely; there is no change for these argument names from the underlying engine. The
...
argument ofset_engine()
allows any engine-specific argument to be passed directly to the engine fitting function, likeset_engine("ranger", importance = "permutation")
.
That would be great! Here is info on contributing documentation to parsnip.
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.
For LDA using the MASS and sparsediscrim engines, the base modelling functions have a
prior
argument.It would be useful if this could be specified in
discrim
/tidymodels
.This PR just adds
prior = NULL
as enginedefaults
for these two engines.I am not sure if this is the most sensible way to do this. Prior probabilities aren't really tunable (well, shouldn't be?) so I don't think they fit as an argument? But it is a little awkward to use
set_engine("MASS", prior = c(1,1,1)/3)
prior to passing through tofit()
when we haven't yet defined the outcome variable.Am happy to adjust accordingly. Thanks for the great package(s)!
Simple way to test:
Side note:
lda()
usesis.missing(prior)
to determine whether the prior was set. This seems to work when I specifyprior=NULL
in defaults, but it was unclear to me that it would work - happy to take pointers!