-
-
Notifications
You must be signed in to change notification settings - Fork 399
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
[WIP] Macro extension interface #522
Conversation
cc @ctjandra |
Haven't started reading this yet, but here is what is needed by JuMPeR (so not much): |
In JuMPChance we need to pass keyword arguments through:
|
Should be feasible; we already do that for |
@@ -801,7 +819,7 @@ macro defVar(args...) | |||
# We now build the code to generate the variables (and possibly the JuMPDict | |||
# to contain them) | |||
refcall, idxvars, idxsets, idxpairs, condition = buildrefsets(var) | |||
code = :( $(refcall) = Variable($m, $lb, $ub, $(quot(t)), "", $value) ) | |||
code = :( $(refcall) = $kernelfunc($m, $lb, $ub, $(quot(t)), "", $value) ) | |||
if symmetric |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So at what point do I block this whole path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you need to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I just throw an error if someone passes in the :SDP type to Variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there should be some mechanism for a nice fallback error message in the case of an unrecognized kwarg or if you only implement a few of the Variable
methods implemented or something.
Would need a way to change all the error messages too. I dunno about this, I see what you're getting at but I'd rather just pull out bits as utility functions than try to do it this way. |
I wonder if we could turn |
Also, JuMPChance |
We would need to document which methods are available for you to provide to the macros; if you don't implement it, you would get a no method error. We should also stipulate that you have your type |
Also, we could dirty up the JuMP code a bit so that we don't use constructor methods for passing along those things, but manually update |
What would the implementation of #481 look like with this new interface? It's hard to see how that can be easily abstracted over. |
I imagine that would get de-sugared into a bunch of calls to
anyway, so I'm not sure I see the problem, except that JuMP claims certain kwargs (e.g. |
But is the |
Not unless it wants helpful error messages. The number of possible methods to implement needs some polishing, and it probably makes sense to change the |
But |
Yes, but it's trivial to add a |
I feel like we're trying to reuse too much machinery that's specific to JuMP variables, it's not a very clean abstraction. |
Do you have a more concrete issue? I think inheritance that gets you vectorized operations and helpful error messages for free is a pretty nice abstraction... |
The inheritance aspect is fine. But if we want a solid extension interface at this level, we should be trying to make all of the JuMP-variable-specific processing just an extension itself instead of being baked into the definition of the abstraction layer. We don't want to start breaking extensions as soon as we tweak how we process |
I'd like to add another general use case. One of the things I'm doing is setting attributes to variables. The current interface for that looks like: for s in SCENARIOS, i in CROPS
DSPsolver.setVarSubproblem(m, x[s,i], s)
end from here. It would be nice if I could use JuMP macros to turn that into something like @setVarSubproblem(m, x[s=SCENARIOS, i=CROPS], s) which requires something fairly general. |
Do we have any planned API changes for extensions for 0.10, or just docs? If it's just docs, that doesn't need to hold up the release. |
Probably just docs at this point. |
This is a preliminary sketch of an interface into the lower-level macro code in JuMP. It seems that most practical cases can be handled by swapping out the inner "kernel" function and leave all the indexing, looping, etc. untouched. For example, in
@defVar
the kernel function would beVariable
, and in@addConstraint
it's eitheraddConstraint
oraddVectorizedConstraint
.The interface would be very simple: to roll your own macro, simply pass along the expression to the
__defVar__
or__addConstraint__
functions, along with akernelfunc
kwarg. So,@defUnc
could becomeHopefully this relatively simple change will allow us to avoid exposing all the complicated code under the scenes to generate the indexing loops and all that, without losing too much expressiveness.