When copying a model, propose to only keep conflict constraints. #1135

dourouc05 · 2020-07-30T23:34:12Z

In line with #1035, a first step to allow a common interface for conflicts (exporting just these constraints). Copying the relevant constraints is a more flexible way to get to that point.

blegat · 2020-08-06T08:38:40Z

I would rather do something more general like passing a filter function. This way, it could also be used for instance to relax integrality. Another example: in jump-dev/Pavito.jl#21, I ended up copying the model without integrality to the continuous solver and copying the model without quadratic and nonlinear constraints to the mip solvers.
So copying with a filter seems to be quite useful and I wouldn't specialize the feature for conflicts only.

dourouc05 · 2020-08-12T01:35:17Z

I've just made this modification, so that the code is now much more flexible. However, it feels much slower than previously…

Add a bit more doc.

dourouc05 · 2020-08-12T01:59:38Z

I now use nothing, it feels much faster than before. The performance penalty for people not filtering anything should be very low with respect to pre-PR state (several branches; short unions are supposed to be quite optimised).

For numbers, before this PR (using the benchmark included in the last test):

(9/10) benchmarking "copy_model"...
done (took 2.352988901 seconds)

And after:

(9/10) benchmarking "copy_model"...
done (took 2.270177901 seconds)

I guess we might say there is no significant performance impact on the most common path.

src/Utilities/copy.jl

blegat · 2020-08-21T16:09:25Z

test/Utilities/copy.jl

+
+    # Perform the copy.
+    dst = OrderConstrainedVariablesModel()
+    index_map = MOI.copy_to(dst, src, filter_constraints=f)


Could you also add a test where the filter filters out all constraints of a given type and the output model does not support the type of constraints that is filtered out ? If they are all filtered out, it shouldn't throw UnsupportedConstraint but it'd be good to check with a test. This is needed in the use case of MIP relaxation where a MIP is copied to an LP that does not support integer variables.

Something like the last commit?
2fa662c

…ered for.

blegat · 2020-08-28T13:40:02Z

test/Utilities/copy.jl

+
+    # Perform the copy. This should not throw an error. 
+    dst = BoundModel()
+    MOI.copy_to(dst, src, filter_constraints=f)


Can you also add a @test_throws ... MOI.copy_to(dst, src) to make sure that BoundModel was setup correctly ?

dourouc05 · 2020-08-28T15:00:50Z

It looks like BoundModel was indeed not so correct… Now, it should be!

blegat

Thanks, I believe that this has many use cases

dourouc05 · 2020-08-28T19:59:04Z

Shall I wait until this is merged before starting a PR to expose this functionality in JuMP, but specialised for conflicts? Otherwise, continuous integration can only fail, even though people would still be able to test on their own machines.

I had a look at relax_integrality, but I don't think it would benefit from a rewrite to use this functionality. Do you have any idea where it might be useful in JuMP?

blegat · 2020-08-29T06:55:38Z

You can wait for MOI to be tagged, it will be quite soon.
Other places are META solvers where you usuallu communicate only one part of the model to tu e underlying solver

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135

@mlubin

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 Don't create a testset for each potential test file. This cleans up the output when running the tests, as there is no output for files that are excluded. Previously, the output was like: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 37.4 seconds. Test Summary: | JuMPExtension.jl | No tests callbacks.jl took 15.9 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 With this patch: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 22.0 seconds. callbacks.jl took 16.8 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 Allow filtering constraints when copying a JuMP model. This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 Add test for copying models with filter. Deal with arrays of constraints. Make tests pass. Typo Add dedicated tests with all container types. Add a convenience function `copy_conflict`. @mlubin feedback. Add a compatibility layer between JuMP.ConstraintRef and MOI.ConstraintIndex. This fixes a pretty serious bug… Doc fixes. More consistency in tests. Add link to `copy_conflict` from `solutions.md` Typo @blegat Test copy_conflict based on MockOptimizer WIP. WIP 2. Move condition in functions. Improve style code, as proposed by @odow. Add to doc. Formatting changes were lost by rebasing. Ditto. Other rebase problem.

@mlubin

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 Don't create a testset for each potential test file. This cleans up the output when running the tests, as there is no output for files that are excluded. Previously, the output was like: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 37.4 seconds. Test Summary: | JuMPExtension.jl | No tests callbacks.jl took 15.9 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 With this patch: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 22.0 seconds. callbacks.jl took 16.8 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 Allow filtering constraints when copying a JuMP model. This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 Add test for copying models with filter. Deal with arrays of constraints. Make tests pass. Typo Add dedicated tests with all container types. Add a convenience function `copy_conflict`. @mlubin feedback. Add a compatibility layer between JuMP.ConstraintRef and MOI.ConstraintIndex. This fixes a pretty serious bug… Doc fixes. More consistency in tests. Add link to `copy_conflict` from `solutions.md` Typo @blegat Test copy_conflict based on MockOptimizer WIP. WIP 2. Move condition in functions. Improve style code, as proposed by @odow. Add to doc. Formatting changes were lost by rebasing. Ditto. Other rebase problem.

@mlubin

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 ---------------------------------------------------------- Don't create a testset for each potential test file. This cleans up the output when running the tests, as there is no output for files that are excluded. Previously, the output was like: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 37.4 seconds. Test Summary: | JuMPExtension.jl | No tests callbacks.jl took 15.9 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 With this patch: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 22.0 seconds. callbacks.jl took 16.8 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 Allow filtering constraints when copying a JuMP model. This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 Add test for copying models with filter. Deal with arrays of constraints. Make tests pass. Typo Add dedicated tests with all container types. Add a convenience function `copy_conflict`. @mlubin feedback. Add a compatibility layer between JuMP.ConstraintRef and MOI.ConstraintIndex. This fixes a pretty serious bug… Doc fixes. More consistency in tests. Add link to `copy_conflict` from `solutions.md` Typo @blegat Test copy_conflict based on MockOptimizer WIP. WIP 2. Move condition in functions. Improve style code, as proposed by @odow. Add to doc. Formatting changes were lost by rebasing. Ditto. Other rebase problem.

@mlubin

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 ---------------------------------------------------------- Don't create a testset for each potential test file. This cleans up the output when running the tests, as there is no output for files that are excluded. Previously, the output was like: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 37.4 seconds. Test Summary: | JuMPExtension.jl | No tests callbacks.jl took 15.9 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 With this patch: Test Summary: | Pass Total Containers | 241 241 Containers.jl took 22.0 seconds. callbacks.jl took 16.8 seconds. Test Summary: | Pass Total callbacks.jl | 22 22 Allow filtering constraints when copying a JuMP model. This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135 Add test for copying models with filter. Deal with arrays of constraints. Make tests pass. Typo Add dedicated tests with all container types. Add a convenience function `copy_conflict`. @mlubin feedback. Add a compatibility layer between JuMP.ConstraintRef and MOI.ConstraintIndex. This fixes a pretty serious bug… Doc fixes. More consistency in tests. Add link to `copy_conflict` from `solutions.md` Typo @blegat Test copy_conflict based on MockOptimizer WIP. WIP 2. Move condition in functions. Improve style code, as proposed by @odow. Add to doc. Formatting changes were lost by rebasing. Ditto. Other rebase problem.

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135

When copying a model, propose to only keep conflict constraints.

2e16c43

dourouc05 added 2 commits August 12, 2020 02:56

Filtering when copying: more generic implementation.

ae3ea4a

Add test.

82baad7

Filter less often.

8192220

Add a bit more doc.

Add benchmark for copy.

1342f9c

blegat reviewed Aug 21, 2020

View reviewed changes

src/Utilities/copy.jl Outdated Show resolved Hide resolved

blegat reviewed Aug 21, 2020

View reviewed changes

src/Utilities/copy.jl Outdated Show resolved Hide resolved

blegat reviewed Aug 21, 2020

View reviewed changes

src/Utilities/copy.jl Show resolved Hide resolved

blegat reviewed Aug 21, 2020

View reviewed changes

dourouc05 added 4 commits August 21, 2020 22:36

Fix prototype.

aa7c311

Typo.

d8c4160

Comment about double copies.

27d3de7

Add test: unsupported constraints are not copied when explicitly filt…

2fa662c

…ered for.

blegat reviewed Aug 28, 2020

View reviewed changes

blegat added this to the v0.9.15 milestone Aug 28, 2020

Add a test specifically for BoundModel.

b002a4a

blegat approved these changes Aug 28, 2020

View reviewed changes

blegat merged commit fa88b46 into jump-dev:master Aug 29, 2020

dourouc05 mentioned this pull request Sep 7, 2020

Ease the computation of IIS? jump-dev/JuMP.jl#1035

Closed

dourouc05 mentioned this pull request Sep 21, 2020

Allow copying models with a subset of constraints jump-dev/JuMP.jl#2339

Merged

dourouc05 added a commit to dourouc05/JuMP.jl that referenced this pull request Nov 5, 2020

Allow filtering constraints when copying a JuMP model.

a77ae0e

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135

dourouc05 added a commit to dourouc05/JuMP.jl that referenced this pull request Nov 9, 2020

Allow filtering constraints when copying a JuMP model.

2547a0a

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135

dourouc05 added a commit to dourouc05/JuMP.jl that referenced this pull request Feb 21, 2021

Allow filtering constraints when copying a JuMP model.

90a9abe

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135

odow pushed a commit to jump-dev/JuMP.jl that referenced this pull request Feb 22, 2021

Allow filtering constraints when copying a JuMP model. (#2339)

51b2a91

This replicates the functionality of MOI: jump-dev/MathOptInterface.jl#1135

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

When copying a model, propose to only keep conflict constraints. #1135

When copying a model, propose to only keep conflict constraints. #1135

dourouc05 commented Jul 30, 2020

Uh oh!

blegat commented Aug 6, 2020

Uh oh!

dourouc05 commented Aug 12, 2020

Uh oh!

dourouc05 commented Aug 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blegat Aug 21, 2020 •

edited

Loading

Uh oh!

dourouc05 Aug 21, 2020

Uh oh!

blegat Aug 28, 2020

Uh oh!

dourouc05 commented Aug 28, 2020

Uh oh!

blegat left a comment

Uh oh!

dourouc05 commented Aug 28, 2020

Uh oh!

blegat commented Aug 29, 2020 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

When copying a model, propose to only keep conflict constraints. #1135

When copying a model, propose to only keep conflict constraints. #1135

Conversation

dourouc05 commented Jul 30, 2020

Uh oh!

blegat commented Aug 6, 2020

Uh oh!

dourouc05 commented Aug 12, 2020

Uh oh!

dourouc05 commented Aug 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blegat Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dourouc05 Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

blegat Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

dourouc05 commented Aug 28, 2020

Uh oh!

blegat left a comment

Choose a reason for hiding this comment

Uh oh!

dourouc05 commented Aug 28, 2020

Uh oh!

blegat commented Aug 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

blegat Aug 21, 2020 •

edited

Loading

blegat commented Aug 29, 2020 •

edited

Loading