Skip to content
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

Deferring evaluation of mutable objects #67

Closed
rdeits opened this issue Jul 26, 2018 · 7 comments
Closed

Deferring evaluation of mutable objects #67

rdeits opened this issue Jul 26, 2018 · 7 comments

Comments

@rdeits
Copy link
Collaborator

rdeits commented Jul 26, 2018

(just continuing our conversation from the QPControl bug)

What about a function defer(x) which would do:

function defer(x, model)
  Parameter(() -> x, model)
end

which would be useful for wrapping mutable objects that should be evaluated "later" rather than eagerly. I realize it's only a one-liner, but it would also remove the need for a let block, so:

let x = x
  Parameter(() -> x, model)
end

becomes:

defer(x, model)
@tkoolen
Copy link
Owner

tkoolen commented Jul 26, 2018

There's also Parameter(identity, x, model).

@tkoolen
Copy link
Owner

tkoolen commented Aug 4, 2018

@schmrlng, re: #68 (comment), the keyword constructor wouldn't work with do syntax, so I think that's a showstopper.

@tkoolen
Copy link
Owner

tkoolen commented Aug 4, 2018

But having an additional outer constructor with a required keyword argument instead of replacing the current outer constructor might be a good idea. To support both 0.6 and 0.7:

Parameter(model; fixedval=throw(MethodError("keyword argument fixedval not assigned"))) = Parameter(identity, fixedval, model)

I think I like that better than defer because this way it's clear it's just another way to create a Parameter.

Opinions (also regarding kwarg name)?

@schmrlng
Copy link
Contributor

schmrlng commented Aug 4, 2018

I would be happy with that. fixedval feels like a bit of a misnomer for my (possibly non-idiomatic) usage, i.e., I have an explicit update_model function that has a bunch of lines like param() .= new_param_value. Maybe fixedref instead? Though that muddies the water since the value passed in is getting wrapped in a Ref; it is not a Ref itself. Maybe startval?

@rdeits
Copy link
Collaborator Author

rdeits commented Aug 4, 2018

I don't particularly like fixedval, but it's not too bad. What about just Parameter(defer, x, model)? We just have to define and export const defer = identity.

@timholy
Copy link
Contributor

timholy commented Sep 22, 2018

API-wise, one thing I mildly dislike about the identity solution is that I'm using this in contexts where I have to update multiple parameters simultaneously:

function create_model(args...)
    X = Matrix{T}(undef, n, m)
    vals = Vector{T}(undef, m)
    function updater(leaf)
        # Do something to calculate both `X` and `vals` from `leaf`
    end
    model = ...
    vars = ...
    Xp = Parameter(X, model) # syntax as in #79
    valsp = Parameter(vals, model)
    # define objective, constraints, etc
    return updater, model, vars...
end

updater basically plays the role of the update function for all the parameters; I call it manually for each new leaf before calling solve!(model). It seems a bit redundant to have to specify identity for each parameter after that. However, this is just an aesthetic thing and not really all that important.

Here leaf is a node in a tree from CoordinateSplittingPTrees.

@tkoolen
Copy link
Owner

tkoolen commented Sep 25, 2018

It seems that this issue is the first annoyance for anybody using the package, unfortunately. I'll think about ways to better support your use case.

In an earlier implementation of this package everything worked in this 'push' fashion: the user would just be responsible for updating all the problem data before calling solve! again. But with more complicated models I got worried that I'd forget to update something, so I prefer the current 'pull' style now.

Off topic, and it might not be helpful in your case because you want to call the update function with the leaf argument, but note that you can also call a Parameter from the update function of another Parameter, which you could even (ab?)use to build something like

using Parametron, Random
model = Parametron.MockModel()
n, m = 2, 2
T = Float64
X = Matrix{T}(undef, n, m)
vals = Vector{T}(undef, m)
dummy = Parameter{Nothing}(model) do
    println("updating X and vals")
    rand!(X)
    rand!(vals)
    nothing
end

p1 = Parameter(x -> (dummy(); x), X, model)
p2 = Parameter(x -> (dummy(); x), vals, model)

so that you get:

julia> p1()
updating X and vals
2×2 Array{Float64,2}:
 0.0495582  0.981912
 0.24826    0.326884

julia> p2()
2-element Array{Float64,1}:
 0.47120493476206926
 0.7678895316375276 

Then the solver still does the work of invalidating parameter values at the beginning of a solve! call.

tkoolen added a commit that referenced this issue Sep 25, 2018
tkoolen added a commit that referenced this issue Sep 26, 2018
Fix #67, sugar for creating Parameters that are updated externally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants