How does VariableInSetRef interact with deletion
- Dominant language
- Julia
- Stars
- 2.5k
- Forks
- 428
- Avg merge
- 18h 10m
- Merged PRs (30d)
- 28
Description
See these two issues found by Claude. They both stem from the question: how does VariableInSetRef interact with variable and constraint deletion?
* Now, if we delete a variable, we should probably delete it from the `model.variable_in_set_ref` dictionary.
* But what if we delete a variable that is an element in a vector set?
* And what happens if we delete the constraint but not the variables from VariableInSetRef?
All of this stems from the fact that we have a fuzzy distinction between `add_constrained_variable(s)` and `add_variable(s)` + `add_constraint`.
One answer is to tidy everything up so that deletion works and `is_variable_in_set` returns `false` if the constraint has been deleted etc.
Another answer is that the documentation stays and we just tell people not to delete, even though in theory they can. The `VariableInSetRef` would still return a constraint reference, even if it has been deleted, and that reference would be invalid.
I need to think about this a bit more.
# variables.md: false claim that a variable-constrained-on-creation constraint can't be deleted
`docs/src/manual/variables.md:1355-1357`:
```
!!! note
You cannot delete the constraint associated with a variable constrained on
creation.
```
This is false — `delete(model, VariableInSetRef(x))` works fine, and
`VariableInSetRef` (documented two lines below) is exactly the handle used
to do it.
## Repro
```julia
using JuMP
model = Model()
@variable(model, y[1:3] in SecondOrderCone())
c = VariableInSetRef(y)
delete(model, c) # succeeds, no error
is_valid(model, c) # false -- genuinely deleted
list_of_constraint_types(model) # Tuple{Type,Type}[] -- constraint fully gone
```
Reproduced with both a bare `Model()` and `Model(SCS.Optimizer)` after
`optimize!`. `y[1]` remains a valid, ordinary free variable afterward.
There's no guard against this in `src/`: `delete(model, con_ref)`
(`src/constraints.jl:563`) just forwards to `MOI.delete` with no special
case for a variable-in-set constraint.
Also checked the cases most likely to be the reason the note was written —
a scalar (non-vector) set, and a set that forces JuMP/MOI to insert a
*variable bridge* (HiGHS has no native support for PSD variables, so
`@variable(model, W[1:2,1:2], PSD)` on a `Model(HiGHS.Optimizer)` is
definitely bridged) — and `delete` succeeds cleanly in both:
```julia
model = Model()
@variable(model, x in Semicontinuous(1.0, 2.0))
delete(model, VariableInSetRef(x)) # succeeds
model = Model(HiGHS.Optimizer) # HiGHS doesn't support PSD -> variable is bridged
@variable(model, W[1:2, 1:2], PSD)
list_of_constraint_types(model) # [(Vector{VariableRef}, PositiveSemidefiniteConeTriangle)]
delete(model, VariableInSetRef(W)) # succeeds
list_of_constraint_types(model) # Tuple{Type,Type}[] -- fully gone
is_valid(model, W[1]) # true -- variable itself survives
```
So the note appears to be unconditionally false, not just false for the
vector-cone case.
## Patch
````diff
--- a/docs/src/manual/variables.md
+++ b/docs/src/manual/variables.md
@@ -1352,10 +1352,6 @@
_[3]
```
-!!! note
- You cannot delete the constraint associated with a variable constrained on
- creation.
-
To check if a variable was constrained on creation, use [`is_variable_in_set`](@ref),
and use [`VariableInSetRef`](@ref) to obtain the associated constraint reference:
````
See also [[issue-024]] — a related bug where `is_variable_in_set` isn't
updated after such a delete.
# `is_variable_in_set` still returns `true` after the variable-in-set constraint is deleted
**Note:** the fix here is the same edit proposed in [[issue-016]] — apply it
once, not twice, if actioning both issues.
`src/variables.jl:1893-1901`, `is_variable_in_set(x::AbstractJuMPScalar) =
haskey(model.variable_in_set_ref, x)`. Deleting the constraint via
`delete(model, VariableInSetRef(x))` (which is documented and works, see
[[issue-023]]) does not remove `x` from `model.variable_in_set_ref`, so
`is_variable_in_set(x)` keeps returning `true` even though
`VariableInSetRef(x)` now points to a deleted, invalid constraint.
## Repro
```julia
using JuMP
model = Model()
@variable(model, y[1:3] in SecondOrderCone())
c = VariableInSetRef(y)
delete(model, c)
is_valid(model, c) # false (correct -- constraint is gone)
is_variable_in_set(y) # true -- BUG, should be false after the delete
```
## Patch
Same root cause and same fix as [[issue-016]] (a stale `variable_in_set_ref`
entry outliving what it points to) — rather than adding cleanup logic to
every place a variable-in-set constraint could be deleted (which risks
missing a case, and an O(n) scan over `variable_in_set_ref` per `delete`
call would be wasteful), make `is_variable_in_set` check validity instead
of just dict membership, in `src/variables.jl`:
```diff
--- a/src/variables.jl
+++ b/src/variables.jl
@@ -1893,8 +1893,14 @@
function is_variable_in_set(x::AbstractJuMPScalar)
model = owner_model(x)
- return haskey(model.variable_in_set_ref, x)
+ d = model.variable_in_set_ref
+ return haskey(d, x) && MOI.is_valid(backend(model), d[x])
end
function is_variable_in_set(x::AbstractArray{<:AbstractJuMPScalar})
model = owner_model(first(x))
- return haskey(model.variable_in_set_ref, x)
+ d = model.variable_in_set_ref
+ return haskey(d, x) && MOI.is_valid(backend(model), d[x])
end
```
Verified: with this patch, `is_variable_in_set(y)` correctly returns `false`
after `delete(model, c)` in the repro above.
Contributor guide
Research direction
Start with src/variables.jl:1893-1901 and the deletion behavior in src/constraints.jl:563, then reproduce the VariableInSetRef examples in the issue. Decide how deletion should affect is_variable_in_set, apply the stated validity check if that behavior is adopted, and remove the false note from docs/src/manual/variables.md. Done means deleted references are invalid and is_variable_in_set returns false.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- backend-api-design, documentation
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 66/100