ModelParameters.jl
ModelParameters.jl copied to clipboard
Make default bounds type Interval from IntervalSets
This is a minor issue, but I think it would make more sense to use IntervalSets.Interval
for the bounds
field of Param
(this goes for FieldMetadata
as well). As far as I can tell, there is nothing that stops you from doing this at the moment, but since the type of bounds
is listed as Tuple
in FieldMetadata
, I assume there is some code somewhere that assumes it will be a tuple of numbers..?
Anyway, IntervalSets
provides nice syntax for closed intervals 0..1
, membership checks with ∈
, automatically handles casting between number types (e.g. declaring the interval with Int
but using it with Float64
values), and of course properly distinguishes between open and closed intervals.
I don't really see any reason not to use it as the standard type for bounds
.
I've just been considering swapping DimensionalData.jl over to that too. I wasn't really aware of the package previously.
But the problem with defaults provided by packages is we have to depend on the package. And we want to keep this as small as possible.
There is code in InteractModels.jl that assumes that assumes bounds is a Tuple
:
https://github.com/rafaqz/ModelParameters.jl/blob/master/InteractModels/src/interactive.jl#L241-L263
Can we generalise that to Intervals without needing the dependency? E.g. just using first
and last
? The main requirement in that case is something we can make sliders from. so a tuple should continue to work... a length 2 vector should probably also work.
IntervalSets
is a pretty lightweight package, I think (only three dependencies and a couple hundred lines of code). So I probably wouldn't worry about depending on it. For most users it will probably already be in the package environment.
But I guess we could generalize it. So one can use any type for bounds
but it must simply provide first
and last
as the bounds? Is that the idea?
I always try not to add a dependency... They add up in compile time and CI run time, and it adds maintenance burden.
Also thinking about this, doesn't Optim.jl take tuple bounds too? Or maybe two vectors. But I don't know if it handles intervals. That's where it comes from originally and why it's the default. I think using Tuple
/two fixed values is the more common use case than Interval
.
Interval
also doesn't seem to define first
and last
. So therye hard to handle genericaly. I guess the open/closed thing complicates that anyway.
I'm not sure how we are meant to handle that for Optim.jl or making sliders from intervals either. You would have to use eps
etc. and make sure it works with all types. I would prefer not to worry about that without a really pressing use-case.
Also thinking about this, doesn't Optim.jl take tuple bounds too? Or maybe two vectors. But I don't know if it handles intervals.
It takes two vectors. So it doesn't really work "out of the box" with the tuples either. You would need to map over the Param
s with first
and last
to produce the upper and lower bounds vectors.
Interval also doesn't seem to define first and last. So therye hard to handle genericaly. I guess the open/closed thing complicates that anyway.
Yeah... I suppose that's intentional. But this is a legitimate problem that should be considered in many cases. Several of the parameters I deal with have open lower bounds, for example (zero to infinity, open on both ends). This is, in my opinion, an advantage of Interval
because it forces you to be explicit about this, whereas the Tuple
is kind of ambiguous. I suppose the general assumption is that it's closed..? I think that's the assumption from Optim
.
Yes, you would have to use eps()
for the open case... but shouldn't you kind of be doing that anyway? If you have a parameter with an open boundary, you by definition should not allow the slider or optimizer to use the exact value of the bound. That seems like a pressing use-case to me!
I think you're right about sliders and sets, not including zero is often very useful.
But using intervals is definitely more complicated than Tuple
... If you want to take that on and make PR I can go with that. But we should make sure that:
- common types work as bounds, remember
Int
doesn't haveeps
but are valid bounds values. -
Tuple
bounds still work for sliders - there is an easy way to get Optim.jl bounds from intervals.
Where is the code currently that actually handles the bounds
field?
https://github.com/rafaqz/ModelParameters.jl/blob/master/InteractModels/src/interactive.jl#L196-L202
Not so elegent, but it works. The idea is that a range
is preferable for a slider, but we can work with bounds too, which are more often used for other things. Bounds for Optim etc are probably the same as the bounds you wan to investigate manually.
withunits
hacks in units from a separate units
field, as sliders can have them while the optimiser probably can't. And it's often nice to keep them separate for other reasons too - the bounds and the main value have the same units and specifying that three times is annoying.
I really should comment that code a bit more
Ok, I'll take a look. It doesn't seem like it should be that hard...
Regarding the different types issue, I think the open/closed distinction only makes sense for real numbers. An open integer set doesn't really make sense because you could just equivalently define a closed set with the next integer. So we don't have to provide method dispatches for open integer sets, I think.