Unique names don't require underscores #161
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #161 +/- ##
==========================================
+ Coverage 94.73% 94.76% +0.02%
==========================================
Files 13 13
Lines 703 707 +4
==========================================
+ Hits 666 670 +4
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/dual_model_variables.jl
Outdated
| @@ -149,6 +149,7 @@ function add_dual_variable( | |||
| func = get_function(primal_model, ci) | |||
| set = get_set(primal_model, ci) | |||
| for (i, vi) in enumerate(vis) | |||
| unique_var = length(vis) == 1 | |||
There was a problem hiding this comment.
This can be moved one line above outside the for loop
src/dual_model_variables.jl
Outdated
| @@ -176,9 +178,14 @@ function set_dual_variable_name( | |||
| i::Int, | |||
| ci_name::String, | |||
| prefix::String, | |||
| unique_var::Bool = false, | |||
There was a problem hiding this comment.
We don't need a default value if we set this value everywhere we use this function. Otherwise that just adds a chance to forget to set this and have a silent bug
There was a problem hiding this comment.
was to avoid breaking things but fine with no default
There was a problem hiding this comment.
I think it's fine, we can always tag v0.6, we haven't hit v1 yet
src/dual_model_variables.jl
Outdated
| @@ -176,9 +178,14 @@ function set_dual_variable_name( | |||
| i::Int, | |||
| ci_name::String, | |||
| prefix::String, | |||
| unique_var::Bool, | |||
There was a problem hiding this comment.
How about ; ensure_unique::Bool = true,.
If this is a public method, then we don't need to break things. Appending a false isn't very informative for the caller.
There was a problem hiding this comment.
sounds good. It's not super important since this is internal anyway
|
replaced by #184 |
All models end up marking each variable group with
_$i, even though the variables themselves are often in containers.This typically ends up with a double indexing scheme in JuMP:
This PR adapts it to avoid the
_iif the variable name is unique