Skip to content

WithTimeLimit trait implemented for LpSolveProblem, MicroLpProblem, CPLEXProblem, and lp-solvers Model<T>#113

Closed
jk2997 wants to merge 18 commits intorust-or:mainfrom
jk2997:main
Closed

WithTimeLimit trait implemented for LpSolveProblem, MicroLpProblem, CPLEXProblem, and lp-solvers Model<T>#113
jk2997 wants to merge 18 commits intorust-or:mainfrom
jk2997:main

Conversation

@jk2997
Copy link
Copy Markdown
Contributor

@jk2997 jk2997 commented Nov 9, 2025

No description provided.

russcip = { version = "0.8.2", optional = true }
lp-solvers = { version = "1.0.0", features = ["cplex"], optional = true }
cplex-rs = { version = "0.1", optional = true }
cplex-rs = { git = "https://github.com/jk2997/cplex_rs_fork", optional = true }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't ship a git dependency. Could you pleas upstream your changes into cplex-rs first :)


impl WithTimeLimit for MicroLpProblem {
fn with_time_limit<T: Into<f64>>(self, _seconds: T) -> Self {
// microlp does not support time limits yet
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then do not implement WithTimeLimit on it :)

let name = CString::new("sos").unwrap();
self.0
.add_sos_constraint(&name, SOSType::Type1, 1, &weights, &variables);
.add_sos_constraint(&name, SOSType::Type1, 1, weights.as_mut_slice(), variables.as_mut_slice());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems unrelated ?

Comment on lines -51 to +61
let (obj_coefs, obj_idx, _const) = expr_to_scatter_vec(objective);
assert!(model.scatter_objective_function(&obj_coefs, &obj_idx));
let (mut obj_coefs, mut obj_idx, _const) = expr_to_scatter_vec(objective);
model.scatter_objective_function(obj_coefs.as_mut_slice(), obj_idx.as_mut_slice()).unwrap();
for (i, v) in variables.into_iter().enumerate() {
let col = to_c(i + 1);
assert!(model.set_integer(col, v.is_integer));
model.set_integer(col, v.is_integer).unwrap();
if v.min.is_finite() || v.max.is_finite() {
assert!(model.set_bounds(col, v.min, v.max));
model.set_bounds(col, v.min, v.max).unwrap();
} else {
assert!(model.set_unbounded(col));
model.set_unbounded(col).unwrap();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why all these unrelated changes ?

Maybe open a separate pr if they are indeed justified ?


#[test]
fn solve_problem_with_time_limit() {
eprintln!("Testing time limit...");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

@jk2997
Copy link
Copy Markdown
Contributor Author

jk2997 commented Nov 10, 2025

The as_mut_slice() changes were because the scatter_objective_function() and add_sos_constraint() methods in rust-lpsolve now take &mut [f64] and &mut [i32] types as arguments instead of Vec<f64> and Vec<i32>. The set_integer(), set_bounds(), and set_unbounded() methods also now return Result<(), LpSolveError> types, so I replaced the assert statements with .unwrap() instead. I will modify the issues and open a new PR if necessary.

@jk2997 jk2997 closed this Nov 10, 2025
@KnorpelSenf
Copy link
Copy Markdown
Contributor

Receiving feedback on a pull request is normal, you can also just leave this open and push to the same branch to fix things up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants