example showing how to use update!(model, table)

[lazarusA]

Hi,
I was wondering if you be so kind to upload an example where a table is used to update the model parameters.
Also, not sure from the documentation: is possible to just update a few parameters? because let's say the table is incomplete or I just need to update some of them.

[rafaqz]

See the tests here:
https://github.com/rafaqz/ModelParameters.jl/blob/master/test/runtests.jl#L109-L114

We should probably document everything better some day.

You cant set anything individually yet, because we are using Flatten.jl to just replace all the parameters at once. But we could write something to flatten the model, change a particular row, then rebuild it. I guess that would be useful. Like update!/update that takes an index or vector of indices?

For now you can use Setfield.jl/Accessors.jl to @set the values of the column tuple to what you want, and update them (untested):

vals = model[:val]
model[:val]  = @set vals[7] = some_newal

If you want to add methods to update! here that does this a PR would be good.

[briochemc]

I almost submitted the same issue a few hours ago, but for the out-of-place update, in which I would like to also be able to only update a few of them, something like:

Base.@kwdef struct MyParams{A, B, C}
	a::A = Param(1.0, optimizable=true)
	b::B = Param(2.0, optimizable=false)
	c::C = Param(3.0, optimizable=true)
end

model = Model(MyParams())

update(model, rand(2), x -> x.optimizable) # does not work (trying to update only the "optimizable fields")

Since this is so close to what I was thinking of, I thought I should just chime in here...

[briochemc]

Just throwing this a bit randomly as a follow-up on a discussion with @rafaqz: Thinking about a generic term to distinguish parameters that one may want to collect separately... What about having FreeParam and a FixedParam types, such that then flattening/reconstructing could select/ignore by dispatching on those?

(As usual, disregard if stupid/useless comment 😅)

[rafaqz]

Yep that will do it!

We could make a bunch of SomeParam <: AbstractParam types for different use cases (the default is already AbstractParam), allowing specific filters to be applied manually.

For now you can just define those yourself until we settle on names:

struct FreeParam{T,P<:NamedTuple} <: AbstractParam{T}
    parent::P
end
FreeParam(nt::NT) where {NT<:NamedTuple} = begin
    _checkhasval(nt)
    FreeParam{typeof(nt.val),NT}(nt)
end
FreeParam(val; kwargs...) = FreeParam((; val=val, kwargs...))
FreeParam(; kwargs...) = FreeParam((; kwargs...))

Then the change we need to make is to the flatten/reconstruct calls:

params(x; select=SELECT, ignore=IGNORE) = Flatten.flatten(x, select, ignore)
stripparams(x; select=SELECT, ignore=IGNORE) = hasparam(x) ? Flatten.reconstruct(x, withunits(x), select, ignore) : x

Also add kw... to pass through all the functions that call these. Then you will be able to control the flattening just like in Flatten.jl if you need to.

Only thing is it looks a bit funny in getindex with brackets:

model[:val; select=FreeParam]

And you should be able to do everything we talked about.

I can put together a PR to do this properly, we could have a @param macro to automate the structs above too if you think thats better than having them predefined. Thats one of the few things macros are still good for.

[lazarusA]

that looks nice. But, one needs to call first Model. Today I needed to update the value of some parameters (and keep all the other fields as well) and I did something ugly 😄 , if a better option is possible please let me know. It needs to be one by one, not all are available at the same time.

function setParam(p::AbstractParam, newVal)
    v = values(parent(p))
    newVals = (newVal, v[2:end]...)
    newTuple = NamedTuple{keys(parent(p))}(newVals)
    return Param(newTuple)
end

[rafaqz]

Oof yes that's ugly. Lets wrap that. It will need some tweaks to work with ConstructionBase.jl (an embarrassing omission on my part) but you should just be able to do @set p.val = newval.

As I said to @briochemc I pretty much left this package in its initial state, waiting for some users with feedback like this as to how to improve it. So things will get better, just not that many people have cared enough yet or pointed out the flaws.

[lazarusA]

Doing @set p.val = newval didn't work for me.

p = Param(2.0, units = "k", bounds = (1, 2))
@set p.val = 1
ERROR: type NamedTuple has no field parent
Stacktrace:
 [1] getproperty(x::NamedTuple{(:val, :units, :bounds), Tuple{Float64, String, Tuple{Int64, Int64}}}, f::Symbol)

[rafaqz]

Yeah that's what I mean... it should but doesn't. We can make it work by definining ConstructionBase.constructorof correctly for Param.

[rafaqz]

@lazarusA I'm still thinking about how best to update parameters by number as well as type here, I think we may as well have both?

[lazarusA]

yeah probably, type will be very useful as well. Also I noticed that as it is now if you do the following

Base.@kwdef mutable struct MyParams{A, B, C}
	a::A = Param(1.0, units=u"kg")
	b::B = Param(2.0, units=u"m")
end
m = MyParams()
@set m.a.val = 2

is not the same as

p = m.a
@set p.val = 1

which does the intended.

[rafaqz]

Works for me?

You need to do:

m = @set m.a.val = 2

Or use @set! with a bang, which does the reassignment automatically.

Setifield.jl returns a new object rather than updating the field, as its made for immutables.

I would just drop the mutable from your struct definition ;)

[lazarusA]

I kinda want to keep the mutable part. Not sure about the overhead of the reassignment when you do this thousands of times and the val fields could be large arrays.
Now, is good to know all these options. I will play around with them.

[rafaqz]

immutable is almost always faster

[lazarusA]

I have another scenario where

Base.@kwdef mutable struct MyParams{A, B, C}
	a::A = Param(1.0, units=u"kg")
	b::B = Param(2.0, units=u"m")
        c::C = Param(missing)
end
m = MyParams()
@set! m.c.val = 2 u"m/s"

in this case it will update the val with the number, including the units, perfectly understandable. But the units should be in their respective field. Currently, I'm removing the units before assigning the values. Not sure if it's worth considering, but this could be another case.

[bgroenks96]

@rafaqz @briochemc As mentioned in #39, I would push back against using type matching to accomplish this. This has the (imho significant) disadvantage of hard-coding into your types what is effectively just a property, as demonstrated by @briochemc 's example. This means that, in order to change which parameters are "optimizable" (sticking with the same use-case), you would need to either go into your code and change Param type, or do some messy successive flatten/reconstruct operations to first select the parameters of interest and then reconstruct your model with their Param types changed. This doesn't really fit well with the existing design, I think.

I would be more interested in simply implementing the idea for update given above that takes a predicate and only updates Params matching the predicate, leaving everything else unchanged. This should be relatively straightforward to accomplish with map and filter, I would think.

TL;DR; I think the concept of selecting which Params you're interested in can and should be achieved using the tabular Model design already available rather than complicating the type semantics.

[bgroenks96]

It's also worth noting that any custom subtypes of AbstractParam would need to (redundantly) follow the current structure of Param by at least providing a field parent with a type that has special fields val and component. Having these kinds of hidden/implicit constraints on subtypes is usually a good sign that subtyping is not the best solution to the problem.

[rafaqz]

Param only has one field, that contains a NamedTuple ?

That redundancy is common to all inheritance in julia. The additional constructors that do all the work can be generalised to AbstractPoint... A macro can define the rest if its really necessary, but this is the total boilerplate:

struct FreeParam{T,P<:NamedTuple} <: AbstractParam{T}
    parent::P
end

But your other points are correct. It's not so great for selecting what is optimisable, (although we can just add paramtype as a table column and it could work in the same pass?). And yes filter or foldl over some function is another option, with its own for/against.

The reason to use types is it's easy to plug in to essentially every function in the package, because that's what Flatten.jl does already. This solution is also easier to get type stable for getting/setting the subset of params quickly, as may be required when optimising.

See the PR #39.

[bgroenks96]

That redundancy is common to all inheritance in julia.

I don't think so. That depends on how you use inheritance. I typically use inheritance to group related types together and to facilitate sharing of method dispatches. I really try to avoid this kind of inheritance where you make assumptions about the actual structure of the types and/or their constructors, as opposed to making assumptions about available method dispatches (e.g. similar/getindex for arrays, arithmetic operators for number-like types, etc).

Yeah, I guess a macro can help ameliorate this, but I think it's better to avoid introducing macros unless really necessary. They obscure the meaning of the code to an unfamiliar reader and add an additional cognitive hurdle to understanding how the package actually works.

[rafaqz]

We already use a method for parent, and I wouldnt call that one field "structure". Its one step above a singleton.

I get your gripe, bit it really is minor. This is very little boilerplate. An alternative is to add another free type parameter to Param, if you really want to avoid the duplication.

The other option is the filter/foldl function. If someone has a good syntax, please share.

[bgroenks96]

My gripe isn't really the boilerplate, I agree that this is minor. It is more that I think this is a misuse of inheritance, and it's not clear to me what the gain is versus using the flexibility Param already provides in defining arbitrary fields.

[briochemc]

If someone has a good syntax, please share.

Pretty sure this is not the correct way of doing this (because a lot goes over my head), but did you mean

map(x -> x.val, filter(x -> x.optimizable, ModelParameters.params(model)))

[briochemc]

Sorry for the word salad coming below, but another thing maybe of interest would be to be able to change the type (with the out-of-place update) of only those parameters that require it.

For example, take a model

m = (
   a=Param(1.0, optimizable=true),
   b=Param(2.0, optimizable=false)
)

where a only is optimizable. Then assume one's simulation calls functions f(a) and g(b). An efficient optimization using some AD will likely will call f(a) with a of a special AD type, e.g., a ForwardDiff.jl dual. I do this by overloading Base.:+(m::Model, v) (where v = [ε] here is a vector of just one element: the dual increment for a) based on update(m, t) where t is a table built by taking the table of the original model m, converting the :val column to the AD type of v, and adding ε to a. This means that the updated b is also dual-valued. In other words the updated model would look like

updated_m = (
   a=Param(1.0 + 1.0ε, optimizable=true), 
   b=Param(2.0 + 0.0ε, optimizable=false)  # note the "useless" dual value here
)

This might then slow down calls to g(b), even though the "dual" part of b is zero (just because operations with the dual-valued type can be more taxing, I think). It would thus be nice to be able to build m with an update function that, if needed, only changes the type of those values it updates.

(I'm guessing this might be all completely unnecessary if one could memoize g so that the derivatives avoid redundant calls when updated_m.b == m.b is the memoization condition and the type is not checked.)

[bgroenks96]

Yes, all fine, the implementation of update that I had in mind:

update(m, [a], p -> p.optimizable)

would have to assume that the given vector matches the length of the parameter subset induced by the predicate. It would internally map over the full Param tuple given by params and update only the params matching the predicate.

There would be issues with making this type stable because obviously the length of the parameter subset is not known at compile time, but I think this might be solvable by creating a ParamSet type or something which applies the predicate on construction and stores the length N in the type parameters.

[bgroenks96]

but did you mean

Yes, that's pretty close to what I meant.

[bgroenks96]

Sorry about that... hit the wrong button.