ykrist / rust-grb

Rust library bindings to the Gurobi optimiser.
MIT License
19 stars 8 forks source link

I'm trying to implement General Constraints of Type Indicator #15

Open ugur-a opened 3 weeks ago

ugur-a commented 3 weeks ago

Hi! First of all, thanks for the crate!

For my use case, I need the General Constraints of type Indicator (is "Generic Indicator Constraints" a better name?). I'd love to contribute an implementation of them, but currently there are some places where I'm not entirely sure how to best incorporate them into existing types. I was curious whether you would be interested in adding them in the near future (maybe you have goals of higher priority right now?). In case you are, here are where I'm so far:

This constraint type has the following form:

<binary_variable> == <binary_value> => <linear_constraint> 

So the final function would look something like this (this is based on Model::add_constr):

pub fn add_genconstr_indicator(
        &mut self,
        name: &str,
        ind: Var,
        ind_val: bool,
        con: IneqExpr,
    ) -> Result<Constr> {
        let constrname = CString::new(name)?;
        let (lhs, sense, rhs) = con.into_normalised_linear()?;
        let (vinds, cval) = self.get_coeffs_indices_build(&lhs)?;
        self.check_apicall(unsafe {
            ffi::GRBaddgenconstrIndicator(
                self.ptr,
                constrname.as_ptr(),
                todo!("binvar -> coef (by using `get_coeffs_indices_build`?"),
                ind_val as ffi::c_int,
                cval.len() as ffi::c_int,
                vinds.as_ptr(),
                cval.as_ptr(),
                sense as ffi::c_char,
                rhs,
            )
        })?;

        Ok(self.constrs.add_new(self.update_mode_lazy()?))
    }

... with ffi::GRBaddgenconstrIndicator looking as follows:

pub fn GRBaddgenconstrIndicator(
        model: *mut GRBmodel,
        name: c_str,
        binvar: c_int,
        binval: c_int,
        nvars: c_int,
        ind: *const c_int,
        val: *const c_double,
        sense: c_char,
        rhs: c_double,
    ) -> c_int;

So, let me know what you think, and whether I should make a (draft) PR so that you could examine the changes more easily. Thanks in advance!

[^1]: docs say you can pass any variable here, and it will be coerced to binary

ugur-a commented 3 weeks ago
todo!("binvar -> coef (by using `get_coeffs_indices_build`?"),

after some more digging, I think this would be done either with Model::get_index or Model::get_index_build. Not entirely sure what the difference between these is and which one I'm supposed to use, but I think it's the latter?

ykrist commented 3 weeks ago

Hi! This is very much in-the weeds. In the C API, Gurobi variables, linear constraints, quadratic constraints, etc are all represented by continguous indices starting from 0. The index manager (IdxManager) is in charge of mapping the Rust types like Var, Constr and friends to the C indices, handling the changes to indices that result from removing and adding constraints. Model::get_index and Model::get_index_build just call the IdxManager methods.

The reason for the different methods is that some operations, like querying attributes, require a call to model.update() first. Other operations, like building a constraint from freshly-added variables, do not require an explicit model.update(). The former will call Model::get_index under the hood, and the latter will call Model::get_index_build. You are right, for adding general indicator constraints, we want Model::get_index_build. (I regret making such shit names)

I mention the C index thing because Var, Constr, QConstr etc all have their own set of indices (so the first Var and the first Constr both have index 0). I think general constraints will be the same, so we'll need to add a new GenConstr model object, as well as a new index manager field to Model.

You could always work around the general constraints by implementing them manually using big-M constraints, assuming you can bound the left-hand side of the inequality.

ugur-a commented 3 weeks ago

EDIT: I've figured out the ffi thing -- the reason the function newly added to grb-sys2 wasn't getting picked up by grb was that the latter still depended on the version of the former uploaded to Crates.io. I overrode it using [patch] by adding the following to Cargo.toml:

[patch.crates-io]
grb-sys2 = { path = "grb-sys2" }

Thanks for the detailed answer!

I've gathered some of the stuff with indices while looking through the crate, but your explanation certainly cleared up some of the more obscure details, so thanks! I think I have general intuition now, so I'll try looking into GenConstr and the IdxManager thing.

As I understand it, add_genconstr_indicator won't work as it's implemented currently -- but it seems that something is wrong in ffi::GRBaddgenconstrIndicator also. Cargo says it can't find it -- which is weird, because I've just placed it right besides the other constraint functions, more specifically right after this one:[^1] https://github.com/ykrist/rust-grb/blob/74a73d876d11fff0848fae409c20bee2bfc90f64/grb-sys2/src/lib.rs#L68-L78 Would you happen to know why that could be? I've tried cargo clean-ing and rebuilding both grb-sys2 and grb (multiple times), but to no avail.

I've read a bit about the big-M constraints, and I have found a way to encode b == 1 => x == z using them -- but what I need is b == 1 => x >= z... which, now that I think of it, I could probably implement using a slack variable? Nevertheless, I think adding a support of them to the crate might be worthwhile.

[^1]: I really should make this into a PR instead of trying to describe everything with words shouldn't I :sweat_smile: