rust-or / rust-lp-modeler

Lp modeler written in Rust
MIT License
95 stars 29 forks source link

Return map of symbolic variables to values in solver.run #35

Closed remysucre closed 5 years ago

remysucre commented 5 years ago

Right now SolverTrait::run returns HashMap<String, f32>. Usually one might hope to do something with the variable in addition to printing it. In that case, returning a map from the symbolic variable object instead of a String would be more helpful. For example, here I maintain a bidirectional map between symbolic variables and a type X, and printout the value associated to the variable once I find a solution.

zappolowski commented 5 years ago

Returning HashMap<?, f32> for all use cases would require to unify of LpBinary, LpContinuous and LpInteger into a single type (I assume enum here?). As far as I understand, this is currently not feasible as these types need different trait implementations e.g. BoundableLp for just LpContinuous and LpInteger but not for LpBinary, which cannot be done currently (see Enum variant types).

Meanwhile you could adjust your mapping to be <String, T> and use the returned variable name for look up (that's what I do).

remysucre commented 5 years ago

What about LpExpression?

jcavat commented 5 years ago

We can imagine how we do it by thinking about how we would use it.

Something like:

let ref a = LpInteger::new("a");
let ref b = LpBinary::new("b");

...

let (solver_status, solution) = solver.run(&problem).unwrap();
if( solution(b) ){ // infered as bool
    let i = solution(a); // infered as f32
    ...
}

solution could be a function instead of a map. Or we can imagine a structure containing the map and returning the specific type : if( solution.get_bool(b) ) ...

In any case, to unify the vars, we have two choices. Create an intermediate enum :

enum LpVar {
    LpInteger(LpInteger),
    LpBinary(LpBinary),
    LpContinuous(LpContinuous)
}

This solution need From/Into implementation to transform struct to the specific enum.

The second solution is a trait trait LpVar {} and implementations for the three structs. This last case should use a visitor pattern to reproduce the matching, cf this snippet.

remysucre commented 5 years ago

Either way sounds good. I think it’s even OK to have the user call LpVar::new_binary(“a”) etc. so that we can just replace the structs with the enum variants. That is, if it’s compatible with the rest of the API

jcavat commented 5 years ago

It won't affect the DSL. I would use this struct only for helper function. I will try to do something like (in pseudo-rust):

fn <R>solution(t: &T): R where T: Into<LpVar> {
  match t.into() {
      LpInteger(lpi) => vars_value.get( lpi.name ) // then transform to i32 and return,
      LpLpBinary(lpb) => vars_value.get( lpb.name ) // then transform to bool and return,
      LpContinuous(lpc) => vars_value.get( lpc.name ) // then transform to f32 and return,
  }
}
jcavat commented 5 years ago

Solution above only works if rust could overload function (ad-hoc polymorphism). Currently Rust cannot do it. I propose a function for each kind of vars.