scipopt/russcip

Expand Solved into multiple states depending on the solution status

mmghannam opened this issue · 2 comments

Maybe as a second iteration, we could improve the API a little bit. Having solve always returning a Model<Solved> and get_best_sol return an Option is a little bit confusing, I think.

Maybe we should have solve return a Result<Model<SolvedOptimal>, SolveError> or something like that ? That would make it easier to know what happened when an optimal solution is not available.

Originally posted by @lovasoa in #29 (review)

Would this be a good first issue?

I've read a bit into the code (model.rs), it seems like splitting Solved up into SolvedOptimal and SolvedUnbounded requires to simply "move the content" of this function

fn _set_best_sol(&mut self) {
    if self.scip.n_sols() > 0 {
        self.state.best_sol = Some(self.scip.best_sol());
    }
}

into the solve-function.

As for the SolveError, one could create something similar to Retcode - of course don't use Retcode exactly (or wrap it), since Retcode can be Okay, which would contradict to the fact that it is an Error.

If you approve this general outline, I would love to work on it in my free time (it's exam season, so it could take a while).
😃

Hi @CodingTil!

Thank you for thinking about contributing to russcip!

Yes, I have thought about this, but the solve process can stop for many reasons (most of them are here), some of them would allow for a solution to exist (although a non-optimal one). The more I think about this the more I feel it is better to leave it as it is, since splitting into different types would only be useful to prevent some method calls at compile time. Since all of them are to represent SCIP's solved state, all methods are allowed for all of them, so I feel this would create unnecessary complication.

Anyhow, I didn't want to disappoint you 😄 so I added some more issues, some of which are marked as good first issue. Take a look and let me know if you're interested in working on any of them.