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

allow anonymous variables in @variable #735

Merged
merged 2 commits into from
May 27, 2016
Merged

allow anonymous variables in @variable #735

merged 2 commits into from
May 27, 2016

Conversation

joehuchette
Copy link
Contributor

No description provided.

@mlubin
Copy link
Member

mlubin commented Apr 25, 2016

Examples?
Note there's a merge conflict

@joehuchette
Copy link
Contributor Author

julia> m = Model();

julia> x = @variable(m)
##7145

julia> y = @variable(m, [1:3])
3-element Array{JuMP.Variable,1}:
 col_2
 col_3
 col_4

julia> z = @variable(m, [i=1:4; isodd(i)])
# printing error...

@mlubin
Copy link
Member

mlubin commented Apr 25, 2016

I like it. Depends on #736 if you want to specify bounds?

@mlubin
Copy link
Member

mlubin commented Apr 25, 2016

Re: the printing, maybe we should just allow these for Array{Variable} and not jump containers at the start? Or is that just a bad idea?

@joehuchette
Copy link
Contributor Author

Yes you'd need #736 for the bounds. I did that first and branched off it, so that's probably the merge conflict.

I think it's a bad idea to split things like that. I'd rather just have ugly printing that doesn't throw an error

@joehuchette
Copy link
Contributor Author

Also I'm not 100% sure the hygiene here is right

@mlubin
Copy link
Member

mlubin commented Apr 25, 2016

Agreed we should support constructing jump containers.

Also I'm not 100% sure the hygiene here is right

Needs tests :)

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 25, 2016

👍

@chriscoey
Copy link
Contributor

I literally just needed this

@chriscoey
Copy link
Contributor

I literally still need this

@joehuchette
Copy link
Contributor Author

Lol I just need to motivate myself to clean this up

@mlubin
Copy link
Member

mlubin commented May 24, 2016

Time to revive this?

@joehuchette
Copy link
Contributor Author

@mlubin @IainNZ @chriscoey wanna take a look now?

@IainNZ
Copy link
Collaborator

IainNZ commented May 26, 2016

Did we settle on full lowerbound/upperbound?
It also looks a bit weird having kwargs for those but not the category, but not a deal breaker

@joehuchette
Copy link
Contributor Author

We can add category as a keyword argument pretty easily. Annoying corner case: what does

x = @variable(m, Int)

do?

@mlubin
Copy link
Member

mlubin commented May 26, 2016

We should add a category keyword argument but doesn't need to be in this PR.
On May 26, 2016 11:45, "Joey Huchette" notifications@github.com wrote:

We can add category as a keyword argument pretty easily. Annoying corner
case: what does

x = @variable(m, Int)

do?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#735 (comment)

@chriscoey
Copy link
Contributor

The category keyword would have been useful for me recently actually. If it's not hard to get in here, it would be nice to have soon. See #741

@joehuchette joehuchette merged commit 6121efb into master May 27, 2016
y = @variable(m, [2:3,1:3], upperbound=1.0, lowerbound=0.0, Bin)
z = @variable(m, [[:red,:blue]], Int)
w = @variable(m, [i=1:4,j=1:4;isodd(i+j)], SemiCont)
# v = @variable(m, [i=1:3,j=1:3], Symmetric, lowerbound = eye(3)[i,j])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The named version was already broken, I have a fix upcoming.

@mlubin
Copy link
Member

mlubin commented May 28, 2016

This breaks extensions, right?

@joehuchette
Copy link
Contributor Author

joehuchette commented May 28, 2016

To the extent they were relying on the unexported macro functions, maybe. I think the changes should be backwards compatible, but who knows.

@odow odow deleted the anon-vars branch December 30, 2018 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants