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

Review #1

Open
odow opened this issue Apr 18, 2018 · 0 comments
Open

Review #1

odow opened this issue Apr 18, 2018 · 0 comments

Comments

@odow
Copy link

odow commented Apr 18, 2018

General comments:

These are mainly observations looking at the examples.

  • The new version of JuMP will make this whole package much easier
    You will be able to have something like the following (just making syntax up, not an actual model)
emp = EMPModel(agent_models = n)

@variable(emp, x[i=1:n] >= 0)
@constraint(emp, [i=1:n], x[i] in AgentModel(i))
@constraint(emp, sum(x) >= 1)

for (i, agent) in enumerate(agents(emp))
    @agentobjective(agent, Min, x[i])
end

@variable(emp, y)
@constraint(emp, (x[1], y) in VariationalPair())
  • I'm not sure that there is need to support Convex going forward. The new JuMP/MOI is converging towards DCP. This removes a whole heap of the conditional code loading.
  • Have you seen https://github.com/StructJuMP/StructJuMP.jl? This also has multiple models with different scoping.
  • Have you seen https://github.com/jalving/Plasmo.jl?
  • The is the approach I would favour is almost identical to Plasmo. There are scoping issues that need to overcome, but these are just Julia problems:
agent_models = [JuMP.Model() for i in 1:2]
for agent in 1:2
    # x belongs to agent_model[i]
    @variable(agent_model[i], x >= 0)
    @objective(agent_model[i], Min, x)
end
# x is now x[2]
emp = EMPModel(agent_models)
# but we can get x[1] via agent_models[1][:x]
@empconstraint(emp, sum(m[:x] for m in agent_models) >= 1)
solve(emp)

or

agent_models = [JuMP.Model() for i in 1:2]
x = Dict{Int, JuMP.Variable}()
for i in 1:2
    x[i] = @variable(agent_model[i], lower_bound = 0)
    @objective(agent_model[i], Min, x[i])
end
# now we have x[1] and x[2]
emp = EMPModel(agent_models, solver=JAMSDSolver())
@empconstraint(emp, sum(x[i] for i in 1:2) >= 1)
solve(emp)

It's then up to EMPModel to combine the models on the back end.

Pedantic comments:

A lot of the code design stems from the limitations of the current form of JuMP and MPB. Hence the need for the MOI redesign. The new JuMP NL implementation splits out the linear constraints from the non-linear constraints which should help resolve

# TODO(xhub) fix this mess ...

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

1 participant