spine-tools / SpineOpt.jl

A highly adaptable modelling framework for multi-energy systems
https://www.tools-for-energy-system-modelling.org/
GNU General Public License v3.0
57 stars 13 forks source link

Incompatibility of convenience functions with nonlinear expressions/problems #300

Closed spine-o-bot closed 3 years ago

spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Sep 20, 2018, 12:37

I just recognized that our beautiful convenience functions cannot been called in nonlinear expressions without additional work. Therefore I already considered the JuMP documentation, which states that those functions need to registered before using (JuMP.register()) The problem it that keyword arguments (e.g. node="LeuvenElectricity") are not allowed.

minimal example:

using SpineModel
using JuMP
using Ipopt

## ------- Export contents of database into the current session ----------------
path_db= "sqlite:///"*string(@__DIR__)*"/"*"data"*"/"*"GC_minimalExample2.sqlite"
# path_db_out= "sqlite:///"*string(@__DIR__)*"/"*"data"*"/"*"GC_minimalExample2_out.sqlite"
JuMP_all_out(path_db)

##  ------------------------ setting up MCP ------------------------------------
m = Model(solver = IpoptSolver())

@variable(m, x, start = 0.0)
@variable(m, y, start = 0.0)

#@NLobjective(m, Min, (1-x)^2 + 100(y-x^2)^2+var_costs(unit="Midload_PowerPlant"))  #doesnt work!
@NLobjective(m, Min, (1-x)^2 + 100(y-x^2)^2) #works

solve(m)
println("x = ", getvalue(x), " y = ", getvalue(y))

# adding a NL constraint
#@NLconstraint(m, x^2 + y^2 == 1) #works
@NLconstraint(m, x^2 + y^2 == demand(node="LeuvenElectricity", t=1)) #doesnt work!

solve(m)
println("x = ", getvalue(x), " y = ", getvalue(y))

I guess SPINE toolbox should also be able to solve some nonlinear problems, right? Any thoughts?

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Sep 20, 2018, 13:27

We should not use "keyword" in Spine Model anyway. That would be hard-coding. We should move things to a level where they are more generic. We could e.g. have a unitGroup of power plants for which you apply the 'var_costs' addition. This would then be a sub-set of units. Another possibility, currently under discussion, would be to use a 'method' that is applied to specific units. That would also result in a sub-set of units that can be applied as a conditional.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Sep 20, 2018, 18:26

@juha_k but using unit as a keyword argument is no more hard-coding than using unit(). I don't really see the difference. I believe the keyword argument adds a lot to readability and we should strive to keep it. I'll take a look at this issue when I have the time.

spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Sep 21, 2018, 07:15

@juha_k with keyword arguments I just meant the way of calling a convenience function not hard coding keywords, e.g. var_costs(unitGroup=...) would also not be possible right now in a nonlinear program.

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Sep 21, 2018, 08:05

Ok, I might be stepping into controversial territory. So this is my view only. Spine Model control decisions should be based on a function. Therefore you should use subsets like unit_with_var_cost instead of unitGroup="mid_merit_units". You might have other sets of units (e.g. from demand side) that would also like to use the same functionality. That's why there should be a layer in between in the database where you can take your "mid_merit_units" and "middling_demand_response_units" and apply the same functionality to both of them. For this I advocate the method approach. We're currently discussing that with the archetypes, but I feel it would be useful to able to do some of that with unitGroups too (but this is controversial). UnitGroups are under the control of the regular modeler and they shouldn't need to typically touch archetypes. However, currently unitGroups are only used in those generic equations.

I realize that the quotes / no quotes does not remove the 'hard-coding', but there is something fishy about them. One reason to use subsets is that those ="foobar" conditionals will mean string comparisons against every set member and that's not typically very fast. @DillonJ would be better in elaborating on the vices of quote based hard-coding.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Sep 21, 2018, 08:11

@juha_k it's not the keyword itself that you don't like, but the string on the right hand side of the expression? I think @steffenkaminski did it that way for illustration. We would be writing stuff like unit__commodity(unit=u) for u in unit(). Does that seem ok to you?

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Sep 21, 2018, 08:16

@manuelma Right, but there were three entangled points there: 1) No string comparisons 2) Generalize to a function (and 'function' in this case means 'purpose') 3) This generalization works best if there is a layer that allows to go from specific ('mid_merit_units') to the general ('unit_with_var_costs') --> link units, archetypes or unitGroups to a method

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Sep 21, 2018, 08:27

I understand now @juha_k I got confused. Yes, I totally agree that we shouldn't use strings but expressions, and these expressions would be powered by our archetype design. But that's another issue, it belongs to the archetype discussion. Here I think we need to find out how to make functions with keyword arguments work with the @NLconstraint macro from JuMP. But yes you're totally right.

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Sep 21, 2018, 08:47

Yes, the general idea is that the model does need to know about units and nodes etc and this is all handled by JumpAllOut... but the model should never care what a particular object is called...

We have transgressed this general rule with the "in" and "out" thing, but in general, it's not a good idea.

spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Sep 21, 2018, 09:42

I totally agree with everything what has been pointed out. Thank you @manuelma for clarification of my point.

Here I think we need to find out how to make functions with keyword arguments work with the @NLconstraint macro from JuMP

*also for @NLexpression and @NLobjective macro.

Is there maybe a way to evalute/execute our convenience function inline, so the different macros would just recognize a scalar value, or list, etc?

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Sep 21, 2018, 10:44

Sorry for going off track. I am finally able to see your real question (but can't answer it).

spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Oct 4, 2018, 08:12

I asked this question to the community: https://stackoverflow.com/questions/52641915/calling-julia-functions-with-keyword-arguments-using-macro

Lets see whether someone has an idea

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Oct 5, 2018, 11:35

It looks like we got a pretty nice answer...

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Oct 5, 2018, 11:36

assigned to @steffenkaminski

spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Oct 5, 2018, 12:25

Yes, thats true. Unfortunately, this implementation doesn't work with julia 0.6.4. I tried it for Julia 1.0.1 there it worked... Also I was able to compile JuMP at 1.0.1. Im not sure though, which parts of JuMP are still not executable on julia 1.0.1

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Oct 5, 2018, 12:35

NREL had a bit of a nightmare with Julia compatibility and their counsel was to stick with a single version throughout...

... in an ideal world I guess... sounds like it might not be practical for us.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Oct 5, 2018, 12:40

I believe we should take that advice and stay on 0.6.4 or below for now. There's issue #51 about moving to julia 1.0, but not too much activity there. Other packages are also taking it easy, notably JuMP who even advices to stay on 0.6.3 for the moment (or at least that was the situation last time I check, a month ago)

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Oct 5, 2018, 12:43

Also worth noting that their project was 1 year duration and ours is 4?

spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Oct 15, 2018, 14:12

unfortunately, there is still a little problem with the function scope:

julia> var_costs()
Dict{String,Any} with 4 entries:
  "midload_powerplant"  => 30
  "baseload_powerplant" => 15
  "peakload_powerplant" => 50
  "res_powerplant"      => 0

julia> @NL(var_costs, unit="baseload_powerplant")
15

julia> [@NL(var_costs, unit=u) for u in unit()]
4-element Array{Void,1}:
 nothing
 nothing
 nothing
 nothing
spine-o-bot commented 3 years ago

In GitLab by @steffenkaminski on Oct 25, 2018, 09:14

I tried to use macros the wrong way. They are not designed to accept any input except an expression. My next approach would be following:

function call:

@NL @NLexpression(m, #someexpression)

definition NL macro:

macro NL(expression)
   #look for all function calls in expression
   #look whether function is registered as NL function in model
   #if not assume that function is convenient function
   #execute convenient function call before expression and insert return value in expression
end

I wont find any time to implement that within the next two weeks, so I just wanted to comment some next thoughts.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Oct 25, 2018, 19:45

@steffenkaminski maybe take a look at the @butcher macro in helpers.jl for inspiration. It does a very similar thing to what you're proposing, but to accomplish an entirely different thing (issue #61). Would be nice to combine both functionalities.

mihlema commented 3 years ago

It seems like this functionality is not on our priority list. Small update on what's (not) possible:

julia> @NLconstraint(n, x^2 <= SpineOpt.unit_capacity(unit=unit(:CCGT),node=node(:BE_EL),direction=direction(:to_node))) #doesn't work
julia> test= SpineOpt.unit_capacity(unit=unit(:CCGT),node=node(:BE_EL),direction=direction(:to_node))
julia> @NLconstraint(n, x^2 <= test) #works

So if we run into the need of using convenience functions in a non-linear constraint (which we currently don't have), we could e.g. call the convenience function outside the @NLconstraint macro.