rust-or / good_lp

Linear Programming for Rust, with a user-friendly API. This crate allows modeling LP problems, and lets you solve them with various solvers.
https://crates.io/crates/good_lp
MIT License
216 stars 35 forks source link

cannot multiply `usize` by `good_lp::Variable` in constraint on integer variable #31

Closed robbiemu closed 11 months ago

robbiemu commented 12 months ago

Im trying to add a constraint to integer variables:

  let mut problem = variables!();
  let ore = problem.add_vector(variable().integer().min(0), system.time_steps);
  let clay = problem.add_vector(variable().integer().min(0), system.time_steps);
  let obsidian =
    problem.add_vector(variable().integer().min(0), system.time_steps);
  let geode =
    problem.add_vector(variable().integer().min(0), system.time_steps);
...
  for i in 2..=system.time_steps {
  ...
    /* The amount of robots of each type on each minute is limited by the materials available to build them until that minute. the total amount of materials collected until that point minus the materials used to build other types of robots, divided by the cost. */
    model.add_constraint(constraint!(
      ore[i]
        >= (system.ore_robot.requirements.ore.unwrap_or(0) * (ore[I - 1] - 1))
          + (system.clay_robot.requirements.ore.unwrap_or(0) * clay[I - 1])
          + (system.obsidian_robot.requirements.ore.unwrap_or(0) * obsidian[I - 1])
          + (system.geode_robot.requirements.ore.unwrap_or(0) * geode[I - 1])
    ));

but I get errors like:

error[E0277]: cannot multiply `usize` by `good_lp::Variable`
  --> src/part1_solver.rs:98:63
   |
98 |           + (system.geode_robot.requirements.ore.unwrap_or(0) * geode[i])
   |                                                               ^ no implementation for `usize * good_lp::Variable`
   |
   = help: the trait `std::ops::Mul<good_lp::Variable>` is not implemented for `usize`
   = help: the following other types implement trait `std::ops::Mul<Rhs>`:
             <&'a usize as std::ops::Mul<usize>>
             <&usize as std::ops::Mul<&usize>>
             <usize as std::ops::Mul<&usize>>
             <usize as std::ops::Mul>

a hint in the error that we need to cast to f64 even for integer problems would be handy.

lovasoa commented 11 months ago

Hello ! good_lp uses 64-bit floats for all coefficient. You cannot always losslessly convert an usize to an f64, and we wouldn't like to silently introduce incorrect coefficients.

If you can guarantee that your usize values will always be in a range that makes their conversion to f64 lossless, you as use my_usize as f64.

Otherwise, if the values in your usize can be arbitrarily large, you can do something like u32::try_from(my_usize) and handle the resulting Result

robbiemu commented 11 months ago

hey @lovasoa I think you may have closed this a mite early! I realize that it is supporting f64 (and even acknowledged as much in the OP) but there's still a problem.

It took me far too long to realize that I could cast to f64. The front matter in the project repo does not cover such a requirement. The documentation does sort of cover it: https://docs.rs/good_lp/1.4.0/good_lp/variable/struct.Variable.html#impl-Mul%3CVariable%3E-for-f64 but interestingly also https://docs.rs/good_lp/1.4.0/good_lp/variable/struct.Variable.html#impl-Mul%3CVariable%3E-for-i32 Something I think we both have missed based on our posts on this thread. There's a reason for that. f64 is the default, and any error message you get when you accidentally or otherwise provide the wrong value will reflect that Mul is not implemented for usize (but is for those two types).

The error message does not indicate the correctly supported requirement. Since the compiler error is considering the use of usize, you don't get any good options. You could save the users of your system a lot of time and heartache by setting the error message.

Alternatively (as that's a bigger change), good_lp could at least highlight the requirement early in the documentation.

--

I'm a happy camper, I got SCIP working with good_lp and solved my problem nearly quickly. This is just a request to improve the documentation and/or error text for good_lp.

lovasoa commented 11 months ago

Hey, you're right, the documentation is indeed lacking in this regard ! Could you open a small PR to improve the doc comments ? I'll then publish a new version so that it is reflected on docs.rs

robbiemu commented 11 months ago

pr for review: https://github.com/rust-or/good_lp/pull/33/files

thanks for the call to action!

lovasoa commented 11 months ago

Thank you for the contribution! It's already in 1.4.1 :)

https://docs.rs/good_lp/latest/good_lp/variable/struct.ProblemVariables.html