-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
ParamSet Conditions should respect $default #265
Comments
We already discussed that several times (e.g. #259) So the solution is: ps$values <- list(y = TRUE, x = TRUE) I vote for: No Bug |
The dependency is supposed to indicate that presence of a certain parameter (in the example |
Your explanation was already pretty clear to me. However, it heavily relies on the statement that the You might be in favor for a fix within the code but it is less trivial than it appears to be. You cannot just send
|
That would need to change then.
whatever = WhateverClass$new()
set.seed(1)
result1 = whatever$evaluate()
whatever$param_set$values %<>% insert_named(whatever$param_set$defaults, .)
set.seed(1)
result2 = whatever$evaluate()
expect_equal(result1, result2) (possibly with some code to weed out parameters with unfulfilled requirements, but this would still be an easy autotest like function) also note we are setting a pretty high bar here, since we don't currently have systematic tests for most of the other metainformation given in paramset, e.g. lower / upper bounds, dependencies,
I don't think it is overly complicated to have the feasibility-check respect the default values as well. The ok = (p1id %in% ns &&
(p2id %in% ns && cond$test(xs[[p2id]]) ||
p2id %in% self$defaults && cond$test(self$defaults[[p2id]]]))) ||
(p1id %nin% ns) |
Suppose the following
ParamSet
:Here the
default = TRUE
indicates that, if the parameterx
is not given, the whole thing should behave likex
is set toTRUE
. E.g.x
could indicate whether a certain feature is used, andy
could then influence the configuration of that feature---y
then depends onx
beingTRUE
, since it does not have anything to configure ifx
isFALSE
.Since the "default" behaviour indicated is that the feature is usually present (everything behaves like
x
isTRUE
) ifx
is not given at all, it makes sense to just set they
parameter and have thex
parameter be "implicitly"TRUE
.This is not what happens, however:
The
ParamSet$check()
function should do anxs = insert_named(self$default, xs)
after theif(!isTRUE(ok))
block. What I am describing above should be added as a test; possibly add more tests.(I believe one of our student assistants could solve this)
The text was updated successfully, but these errors were encountered: