Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider transforming Status to Result #40

Open
jcavat opened this issue Oct 23, 2019 · 4 comments
Open

Consider transforming Status to Result #40

jcavat opened this issue Oct 23, 2019 · 4 comments

Comments

@jcavat
Copy link
Collaborator

jcavat commented Oct 23, 2019

And putting a solution only for Optimal and Suboptimal.

This would avoid testing the context in many situation.

@jcavat
Copy link
Collaborator Author

jcavat commented Oct 23, 2019

Optimal/Suboptimal for something ok and grouping Infeasible/Unbounded and Notsolved into one case containing error message details.

@zappolowski
Copy link
Contributor

zappolowski commented Oct 23, 2019

Does this mean you're planning to treat Infeasible, Unbounded and/or Notsolved the same way as failing to read/write files or executing the solver? Or did you mean using Either instead of Result (which could be one interpretation of your 2nd comment) like:

Result<Either<WithSolution, WithoutSolution>, String>

I wouldn't aggregate status at this point already as this restricts the freedom of the consumer of this crate. Especially if the error cases are converted to String then, proper error handling will be messy and fragile (due to the need to match on these strings).

@jcavat
Copy link
Collaborator Author

jcavat commented Oct 28, 2019

I would prefer one of these solution :

Result<Solution, Error>
  • with NoSolution defining the Error trait and replace current String by Error and
  • Solution having a bool: is_optimal inside

Create our own Result with somethink like (or a variant of) :

enum Result {
  Optimal(Solution),
  SubOptimal(Solution),
  SolutionError(String),
  Error(Error)
}

I would prefer solution 1 for my part

@zappolowski
Copy link
Contributor

zappolowski commented Oct 30, 2019

The first proposal would mix errors originating from actually trying to run the optimization (e.g. missing optimizer executable, unable to write either input or output files) with the infeasibility to find an optimal solution, which might in some cases be a valid outcome as well (e.g. for distribution problems Infeasible would indicate that the number of agents needs to be increased).

For the second proposal I see problems in error handling. I'm not sure, if it's easy to use ? in that case.

My proposal would be something like:

Result<Outcome, SomeErrorType or String>

with

enum Outcome {
    Optimal(Solution),
    SubOptimal(Solution),
    Infeasible,
    Unbounded,
}

(Outcome is the best I can come up with right now ... I'm bad at naming).

That way, one could use the standard error handling mechanisms and still has more fine grained control on how to interpret the actual optimization result. Usage in that case would roughly look like

let solution = match solver.run(&problem)? {
    Optimal(solution) | SubOptimal(solution) => solution,
    _ => return Err("boo"),
}

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

No branches or pull requests

2 participants