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 more of the newly named functions to take vector arguments #740

Closed
chriscoey opened this issue Apr 26, 2016 · 20 comments
Closed

allow more of the newly named functions to take vector arguments #740

chriscoey opened this issue Apr 26, 2016 · 20 comments
Milestone

Comments

@chriscoey
Copy link
Contributor

So I would never know which of the functions like getvalue, setvalue, setlowerbound etc take single variables and which can take vectors. getvalue will take a vector, setlowerbound doesnt. setvalue, i actually forget, and the docs don't help.

So currently i'm writing things like

    @variable(mip_model, x_orig[j in 1:num_vars_orig])
    for j in inds_bint
        setcategory(x_orig[j], m.types_var_orig[j])
    end

And
map((ind -> setupperbound(x_orig[ind], 0.)), inds_cone)

That's not a big deal that I have to write extra, but what bothers me is that if some of these functions take vectors, why don't all of them?

@chriscoey
Copy link
Contributor Author

Related to the first example I gave, and maybe a separate issue that I can file, it would be better if I could do this:

    @variable(mip_model, x_orig[j in 1:num_vars_orig], m.types_var_orig[j])

But as @mlubin has told me, this doesn't work. I guess the scope of the j index goes to lower and upper bounds, but not to variable type?

@mlubin
Copy link
Member

mlubin commented Apr 26, 2016

if some of these functions take vectors, why don't all of them?

Because nobody was bothered enough to implement them. JuMP never claimed to be feature complete

@chriscoey
Copy link
Contributor Author

it was a half-rhetoric question haha

@mlubin
Copy link
Member

mlubin commented Apr 26, 2016

it would be better if I could do this

That's not how @variable works, see the discussion at #105

@chriscoey
Copy link
Contributor Author

chriscoey commented Apr 26, 2016

OK, yes @variable confuses me sometimes.

Is this related to #736 at all? Like I guess I assumed that it allows us to use the j index in kwargs? (There aren't examples yet so I don't know).

@mlubin
Copy link
Member

mlubin commented Apr 26, 2016

You can use the index j in kwargs. Variable categories aren't kwargs though (they will be at some point in the future).

@chriscoey
Copy link
Contributor Author

so should I submit a second issue for making variable categories kwargs? it's different from this issue about vector arguments

@mlubin
Copy link
Member

mlubin commented Apr 26, 2016

Sure, it's on the roadmap though

@chriscoey
Copy link
Contributor Author

is it a big change? given that we are changing names, functions a lot in the next version, maybe it should go in there? Better to change a lot of things at once and ask people to update their code once

@mlubin
Copy link
Member

mlubin commented Apr 26, 2016

I'm not in favor of rushing to stuff the next release with features given the deadlines and everyone being busy. Right now all the updates we're asking people to do are find/replace changes, and there's a big mental hurdle to ask people to start changing syntax, so I'm not convinced of the benefit of doing it all at once anyway.

@chriscoey
Copy link
Contributor Author

That makes sense. Find and replace is different to changing arguments.

@chriscoey
Copy link
Contributor Author

hey @joehuchette @IainNZ , thoughts on this one? is it tough? can it make it into this new version?

@joehuchette
Copy link
Contributor

Which vectorized methods would you like to see added, specifically? I'm alright adding something like setcategory(::Array{Variable},::Array{Symbol}).

@chriscoey
Copy link
Contributor Author

So how about any of these variable get/set functions that aren't vectorized yet: setname, getname, setlowerbound, setupperbound, getlowerbound, getupperbound, getvalue, setvalue, getdual, setcategory, getcategory, getvariable?

@mlubin
Copy link
Member

mlubin commented Apr 27, 2016

Go for it, except vectorized getvariable doesn't make sense. getvalue and setvalue are already implemented in JuMP.jl.

@IainNZ
Copy link
Collaborator

IainNZ commented Apr 27, 2016

getvariable - probably not that one!

I definitely see the point. What happens with things indexed by 0:10 or random keys, e.g.

@variable(m, x[-5:5])
setvalue(x, ?)

@mlubin
Copy link
Member

mlubin commented Apr 27, 2016

We could allow only a broadcasting version in that case, e.g., setvalue(x, -1), but I'm not sure that's the case that @chriscoey cares about.

@joehuchette
Copy link
Contributor

Yes, I'm (much) less OK adding something like setvalue(::JuMPArray, ::Any).

@chriscoey
Copy link
Contributor Author

the Julia dot syntax should work sometimes, but maybe not for JuMPContainers. so we should test that

@chriscoey
Copy link
Contributor Author

@mlubin said we may drop 0.5 next week-ish. so this code could be moved to JuMP then

@mlubin mlubin mentioned this issue Sep 16, 2017
3 tasks
mlubin added a commit that referenced this issue Sep 17, 2017
Replace JuMPDict with Dict. Rewrite JuMPArray to be compatible with
AbstractArray. Explicit keyword argument in macro to force container
type.

Closes #1099
Closes #1047
Closes #417 (collect is now well defined for Array, JuMPArray, and Dict)
Closes #833 (`eachindex` and `indices` are defined for JuMPArray)
Closes #740 (dot broadcast syntax is now the default, no need to explicitly define vectorized functions)
Closes #922 (fixed by checking for duplicates)
Closes #933 (corollary: closes #346)
mlubin added a commit that referenced this issue Sep 19, 2017
Replace JuMPDict with Dict. Rewrite JuMPArray to be compatible with
AbstractArray. Explicit keyword argument in macro to force container
type.

Closes #1099
Closes #1047
Closes #417 (collect is now well defined for Array, JuMPArray, and Dict)
Closes #833 (`eachindex` and `indices` are defined for JuMPArray)
Closes #740 (dot broadcast syntax is now the default, no need to explicitly define vectorized functions)
Closes #922 (fixed by checking for duplicates)
Closes #933 (corollary: closes #346)
Closes #643 (colons work for Array and JuMPArray, obviously not Dict)
Closes #730 (end is not supported for JuMPArray)
Closes #646 (we now rely on built-in iteration behavior for Dict)
mlubin added a commit that referenced this issue Sep 19, 2017
Replace JuMPDict with Dict. Rewrite JuMPArray to be compatible with
AbstractArray. Explicit keyword argument in macro to force container
type.

Closes #1099
Closes #1047
Closes #417 (collect is now well defined for Array, JuMPArray, and Dict)
Closes #833 (`eachindex` and `indices` are defined for JuMPArray)
Closes #740 (dot broadcast syntax is now the default, no need to explicitly define vectorized functions)
Closes #922 (fixed by checking for duplicates)
Closes #933 (corollary: closes #346)
Closes #643 (colons work for Array and JuMPArray, obviously not Dict)
Closes #730 (end is not supported for JuMPArray)
Closes #646 (we now rely on built-in iteration behavior for Dict)
@mlubin mlubin closed this as completed in 852a3af Nov 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants