-
-
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
paradox/set6 integration #345
Comments
Things that seem to be missing from set6 objects that we use in paradox:
also, how sure are we that param6's |
What does convert do? If qunif is needed it can be added easily but not sure if it's appropriate in the package ? And how does tol work in ParamDbl? Edit: Looked at code for convert and tolerance. It seems this is another point why set6 is useful. ParamDbl with tol=0 is equivalent to a set6 interval with closed bounds, whereas with tol>0 this is equivalently to an interval with open bounds (can also be half-open). So all we need is to make the tolerance amount controllable as either a global option (currently) or local parameter in the Interval object. About convert, I don't fully understand why this is needed as either an element should or shouldn't be in a set. Also I think that conversion, if it is needed for any reason, should really take place when the value is set immediately and not as a method. If needed we can add it easily. And about |
AFAICT |
the way I understand 'open bounds' in set6 is that it makes the set smaller by some small number (if you're lucky enough and your bounds are not so large that machine precision eats that number). What |
Does anything make any use of the constraints in |
even there we don't need it that much |
Got it, we could easily add this to set6.
I guess it asks the paradox question: can this parameter be set, as well as: if it can be set then how can it be set. And the check is only triggered if the value is non-NULL, so it has the same use-case as paradox, i.e. "let this value be set if a condition on the dependency is satisfied". This is still much faster than a general check over the full parameter set |
So speed is the only reason here? Or is |
@berndbischl @mllg @mb706 @be-marc summarising our discussions here and including action points.
Problem: Want to avoid two R6 parameter interfaces that perform a similar task (paradox and param6). param6 created by me as paradox did not work for use-cases that involved more complex sets and had significant bottlenecks.
Solution: Change R6->S3 for Param object in paradox and internally all Params wrap set6 objects. mlr packages will all use sugar functionality to make it easier to develop internal changes without breaking.
Comments: Given the size of the to-do list below and other use-cases paradox cannot currently handle, for example extracting parameters with the same IDs but ignoring prefixes (essential to distr6), I think we should consider converging in the 'other direction'. i.e. start with param6 (if needed we can change the name to paradox) and converge towards paradox, but I currently think param6 actually has more functionality so it could be quicker to change variable names and add sugar functions to param6, than it would be to change underlying objects in paradox.
If we agree that the final package is still called paradox and has at least the same amount of functionality as paradox, then it could be much quicker to start from the param6 code.
To-do (not complete):
ParamX
classes, just have a singleParam
class - RaphaelParam
from R6 to S3 - RaphaelParam
which are in properties/traits of a Set - RaphaelParam
to set properties/traits - RaphaelCondition
from R6 to S3, this can copy cnd from param6 -- Note there are a few conditions in param6 not handled by paradox that are essential in other use-cases, for example checking a parameter value relative to anotherThe text was updated successfully, but these errors were encountered: