search

Gurobi / gurobipy-pandas

Convenience wrapper for building optimization models from pandas data
https://gurobipy-pandas.readthedocs.io/en/stable/
Apache License 2.0
83 stars 15 forks source link

Docs and typing on gurobipy_pandas.api.add_constrs should indicate that LHS and RHS can be single values or floats #77

Open Dr-Irv opened 2 weeks ago

Dr-Irv commented 2 weeks ago

Let's say you have the following:

m = grb.Model()

onevar = m.addVar(name="B")

Bser = gppd.add_vars(m, pd.Index([1,2,3]), name="Bser")

gppd.add_constrs(m, onevar, grb.GRB.GREATER_EQUAL, Bser, name="BLT")

gppd.add_constrs(m, Bser, grb.GRB.LESS_EQUAL, 3*onevar + 4, name="RHSexpr")

m.optimize()

The code above works fine. The first constraint represents the constraint that "B >= Bser", i.e., one variable is greater than all the ones in the series. The second constraint represents that "BSer <= 3*B + 4".

The docs and typing don't say you can do this, but I think you can put any linear expression, including a single variable, or a single value as the rhs or lhs arguments, and it will work fine.

The typing accepts a single value, but the docs don't have that.

simonbowly commented 2 weeks ago

On the order of arguments: I had kept the order strict in typing/docs intentionally (the thinking being for consistency in user code, if you have one side being a series and the other a scalar, putting the series first seems to makes sense). That it works both ways at the moment is really an implementation detail, but we can make it official.

I do think passing scalars for both lhs and rhs should be explicitly disallowed though (such constraints should go through the regular gurobipy API). Would it make sense to document it as "at least one of lhs/rhs must be a pandas series"? For typing we then need three overloads to capture the options.

On the accepted scalar types: yes, this should be updated to accept any scalar (var/linexpr/etc as well as a float).

Dr-Irv commented 2 weeks ago

On the order of arguments: I had kept the order strict in typing/docs intentionally (the thinking being for consistency in user code, if you have one side being a series and the other a scalar, putting the series first seems to makes sense). That it works both ways at the moment is really an implementation detail, but we can make it official.

The docs have the order explicit, but the typing hints do not. I think it should be official that you can do it either way.

I do think passing scalars for both lhs and rhs should be explicitly disallowed though (such constraints should go through the regular gurobipy API). Would it make sense to document it as "at least one of lhs/rhs must be a pandas series"? For typing we then need three overloads to capture the options.

While it ideally wouldn't make sense for someone to do something like gppd.add_constrs(m, 3, grb.GRB.LESS_EQUAL, 4), it could be that they are doing gppd.add_constrs(m, exp1, grb.GRB.LESS_EQUAL, exp2), where exp1 and exp2 are the result of some computation in the code that ends up being a constant unexpectedly. So what would be useful in this case is a warning saying that you passed 2 constant values for the LHS and RHS. I do agree that it makes sense for gurobipy-pandas to require either side to be a Series.

On the accepted scalar types: yes, this should be updated to accept any scalar (var/linexpr/etc as well as a float).

Great.