-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Document cost of named variables and constraints in Performance Tips #2973
Comments
Yeah, it's insane that storing a name in a dictionary can have this impact. I've been hesitant to document because it feels like it should be something we can fix, but I've had a few looks without success. Perhaps we need a Then you could write |
Yeah, i like that idea for a few reasons:
The imagined behaviour i'm talking about in the last point is: julia> @variable(model, foo[z in 1:2, t in 1:3], base_name=nothing));
julia> foo
2×3 Matrix{VariableRef}:
_[1] _[3] _[5]
_[2] _[4] _[6]
julia> model[:foo] == foo
true avoiding the |
Yes, exactly. All it would do is avoid the |
I also prefer "base_name" taking a Bool instead of a Nothing. |
When building a model with millions of variables and constraints, I see model build time roughly half when switching to using anonymous variables and constraints.
An example model has size
And an example code change would be:
Changing all
@variable
and@constraint
calls similarly, I see differences in model build time like below:Depending on the model we're building, the time speed up varies (~1.3 - 2x) but the reduction of allocations is pretty consistently about a ~2x reduction.
The different types of "names" and the behaviour of the macros regarding names is documented
So actually making this change was fairly easy. And this discovery is a very welcome performance improvement!
However, i don't think this significant performance consideration is mentioned anywhere in the docs. And in particular it does't seem to be mentioned in the Performance Tips
I think the overhead of named variables/constraints is known (xref #2817 (comment)), so I think worth documenting given it can be a significant build-time cost and is avoidable with relatively small user code changes.
(p.s. i'm opening this just before some time off so may not be quick to respond)
The text was updated successfully, but these errors were encountered: