jump-dev / jump-dev/JuMP.jl

How does VariableInSetRef interact with deletion

Open
#4,243 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.