Add interfaces

[matthewcrews]

Before I merge this I'd like to get some quick feedback. There are two approaches to the modeling of ILinearExpression. One is to model it as a sequence of elements like this:

[<RequireQualifiedAccess>]
type LinearElement =
    | Constant of float
    | Decision of coefficient:float * decision:Decision

type ILinearExpression =
    abstract member Elements : seq<LinearElement>

I don't like this because it puts a lot of burden on the people interpreting the ILinearExpression to write code which will not blow a stack will iterating through a potentially long sequence.

I prefer this approach:

type ILinearExpression =
    abstract member Decisions : ISet<IDecision>
    abstract member Coefficients : IReadOnlyDictionary<IDecision, float>
    abstract member Offset : float

Which is much easier to use for someone writing a Solver backend. This is the form I prefer to work with when mapping from the the Flips.Model to the Solver model.

[smoothdeveloper]

few thoughts:

ILinearExpression.Decisions may be redundant or confusing if it is the same set as ILinearExpression.Coefficients.Keys.

Also, the current form is convenient for other use case, like pretty printing of terms.

Maybe you should preserve the current type and have seq<LinearElement> -> ILinearExpression transformation function, which is done once when constructing the expression objects?

[matthewcrews]

So you're okay with leaving the burden of interpretation up to the Solver implementer? Like I said, my concern is with someone blowing the stack easily.

[smoothdeveloper]

Like I said, my concern is with someone blowing the stack easily.

Sorry, I may overlook what is the workflow around that, if when I input expression using the operators: someDecision1 * 2.4 + someDecision2 * 4.2 it gives me an seq<LinearElement>, it may be represented as "lazy" ILinearSequence or one in the making, which materializes in the solver into a "reduced" ILinearExpression whenever the model is given for solving.

I'm not clear when the transformation happens, I assume it can happen close to running the solver before registering the constraints / objectives.

I've had to use the current LinearExpression.Reduce to avoid the issue you mention.

[smoothdeveloper]

It overal looks good to me, I see it will replace lots of the internal records in the dev branch.

[matthewcrews]

I decided to go with an ILinearExpression of

type ILinearExpression = 
    abstract member Terms : seq<LinearTerm>

I am considering whether it would be worthwhile to include an ISimplifiedLinearExpression of the form:

type IReducedLinearExpression =
    abstract member SimplifiedTerms : IReadOnlyDictionary<IDecision, float>
    abstract member Offset : float

And a helper function for mapping from ILinearExpression to ISimplifiedLinearExpression. The reason for this is because I find it easier to work with an ISimplifiedLinearExpression when writing the code to interop with a Solver. Without this, I would have to implement the simplification logic in each Solver backend. I'm going to see how much trouble it is before I commit to that. A small amount of duplicate code is fine. Putting the ISimplifiedLinearExpression interface in the Flips.Solver library may make more sense.

The names of types are not final. I was exploring how to implement the interfaces but I plan on going back and making the names of types more clear.

[matthewcrews]

@smoothdeveloper So I think I got this to the point where I feel I can merge this into dev. Normally I would NOT like a PR to be this large. The reason it ended up that way was because I wanted to migrate all the code to the interfaces and get the tests and examples running again.

I'd appreciate you giving this a once over to make sure I haven't missed something obvious. I think this will allow us to start migrating solvers to the interfaces. I'd be happy to help migrate your CPLEX code since that's what we use. I'm going to start filling out the features for CBC and GLOPS.

[smoothdeveloper]

Normally I would NOT like a PR to be this large. The reason it ended up that way was because I wanted to migrate all the code to the interfaces and get the tests and examples running again.

I'm perfectly OK with the PR and motives, this is a first take of splitting some of the model type representation from the API with the idea we should use interfaces to facilitate a broad axis of extensibility.

I think as we mature the rest of the API and see more extensive samples, use case specific circumstances would benefit such extensibility, as an easy to reason escape path when some of the choices made for the default representation hits a wall.

Also, since this PR was open, no other suggestions really came to me or others to help you explore more options.

Merging this PR will probably enable me to do a small round of adjustments in the CPLEX backend.

[smoothdeveloper]

Regarding the technicals of the merge, since there is a little bit of noise in the diff (some whitespace adjustments, commented code) and there has been significant edits that are potentially conflicting in main branch, I'd suggest to proceed like this:

• rebase dev on top of main (I think there is only one PR of mine that I'll need to rebase, but it may impact some private branches for other people)
• open another PR with the minimum changes of introducing the interfaces but with none of the tweaks to some of the code that @TysonMN has adjusted in main, merge that separate PR
• see whatever is missing in the API surface from the changes in this PR, possibly, minimize the diff by removing all the places where there is conflict (because we are in better place reevaluating the rebased dev from scratch about missing adjustments that would have slipped)

[matthewcrews]

I tried rebasing dev onto main last night and the project files got really messed up for some reason. Cannot tell you why. The dev branch here is now messy. I tried fixing it but there's a lot of project settings toggling in the .fsproj files that I'm not used to. I am not as familiar with rebasing so I'm sure I did something wrong due to an incorrect understanding.

Any insight you can offer would be appreciated. I've mostly done merges in the past.

[TysonMN]

I tried rebasing dev onto main last night and the project files got really messed up for some reason. ... I've mostly done merges in the past.

Generally speaking, I prefer rebasing when there are no conflicts and merging when there are. Do you feel like you have the correct code after merging?

[matthewcrews]

So far the code looks good. The .fsproj files are what need fixing.

[TysonMN]

Ok, I will take a look soon.

[TysonMN]

I tried merging main into dev. There were conflicts in eight files but none of them were in project files. Can you merge main into dev, resolve the conflicts in those eight files, and then share that work in a branch? At that time, I can see what issues remain in the project files.

[matthewcrews]

I honestly can't figure out what happened. The .fsproj files are so messed up I can't even fix them. They are using a lot of toggles that I've never used before so I have a really hard time debugging the error messages I get from the build. I have half a mind to just nuke the dev branch since it appears to be so mangled at this point. I've spent several hours trying to fix it and I'm not getting anywhere.

I have merged main into dev. The .fsproj files are really messed up for some reason. Lots of duplicates. That don't make sense.

[matthewcrews]

Yeah, dev looks so bad at this point, I don't know if it's salvageable. Code formatting got mangled and I can't figure out how to revert the rebase.

[matthewcrews]

I'm going to try to undo the rebase and try over

[matthewcrews]

So, I think I was able to revert the rebase and I'm trying to merge main into dev. I'm not a git wizard so I'm learning as I go. All the formatting changes didn't help but that's part of why this needs to be done 🤷‍♂️.

I reverted to e54f8c5 and am attempting the merge. There's going to be some patching though due to formatting changes.

[matthewcrews]

I was able to revert the rebase, merge main into dev, and then patch up the conflicts between dev and add-interfaces. It's building on my machine now using the build.cmd script. @TysonMN could you see if the dev branch is working for you now?

[TysonMN]

Executing sh build.sh seems to work. Building in Visual Studio fails only for Flips.Solver.Cplex because the dependent assemblies ILOG.Concert and ILOG.CPLEX are missing.

[smoothdeveloper]

@TysonMN good point, we are going to need to make non redistributable solvers opt-in options in the build script.