Skip to content

Conversation

blegat
Copy link
Member

@blegat blegat commented Mar 9, 2021

Follows what the comment says.

@odow
Copy link
Member

odow commented Mar 9, 2021

See the test failures. Is this breaking?

@blegat blegat force-pushed the bl/index_map_varmap branch from 984e5b3 to ca557be Compare March 11, 2021 22:42
@blegat
Copy link
Member Author

blegat commented Mar 11, 2021

As briefly mentioned in #1245 (comment),
there is an issue with the use of CleverDicts for constraint indices.
By default, CleverDicts uses an anonymous function for inverse_map but that is not an option if you want
to know the concrete type of the CleverDict.
What Gurobi, etc... do is to define a function that maps Int to variable indices and use the type of that function.
For VectorOfConstraints you cannot do that as it depends on the type of the constraints.
Currently, we use Fix1 for that but it does not play well with inference because the constraint type is stored as a DataType:

Base.Fix1{typeof(CleverDicts.index_to_key),DataType}

This PR provides a non-breaking change that do not create the anonymous function by default and is non-breaking as if the user provides a custom function, the behavior is unchanged.

@blegat blegat requested a review from odow March 11, 2021 22:53
Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

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

Nice. Does this improve the stress_precompile benchmark?

@blegat blegat removed the breaking label Mar 13, 2021
@blegat blegat force-pushed the bl/index_map_varmap branch from 77a9ace to 83da6ae Compare March 13, 2021 09:53
@blegat
Copy link
Member Author

blegat commented Mar 13, 2021

With JuMP master, with the benchmark of jump-dev/JuMP.jl#2484
Before this PR:

julia> @time Foo.example_diet()
 18.208597 seconds (26.08 M allocations: 1.303 GiB, 4.28% gc time)

After this PR:

julia> @time Foo.example_diet()
 17.757552 seconds (25.81 M allocations: 1.291 GiB, 4.34% gc time)

@blegat blegat merged commit 26adf96 into master Mar 13, 2021
@blegat blegat deleted the bl/index_map_varmap branch March 13, 2021 15:25
@blegat blegat added this to the v0.9.21 milestone May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants