Allow stacking of bridge optimier#3013
Draft
blegat wants to merge 5 commits into
Draft
Conversation
Foundation working (Variable/map.jl + bridge_optimizer.jl):
- Variable.Map has outer_to_inner / inner_to_outer IndexMaps, next_outer_variable counter, per-(F,S) constraint counters
- activate_variable_mapping!(map, model) / activate_constraint_mapping!(map, model, F, S) — lazy triggers, called from
add_constrained_variable(s) when a variable bridge is created
- _record_inner_variable!(b, inner_vi) — translates inner→outer when mapping active, identity otherwise
- MOI.add_variable / MOI.add_variables / MOI.add_constrained_variable(s) updated to translate
Verified manually:
- Basic single-layer flow: bridged variables (xs = [-1, -2, -3]) still use slot/negative scheme internally. Non-bridged passthrough
variables get outer indices via IndexMap when active.
- Two stacked SingleBridgeOptimizers succeed at add_constrained_variables — the outer pulls inner-allocated indices (positive) through
_record_inner_variable! while its own bridges keep negative outer indices.
Test status on test/Bridges/General/test_bridge_optimizer.jl:
- test_CallbackVariablePrimal errors — MOI.get(b, attr, vi) doesn't translate vi outer→inner yet
- test_nesting_SingleBridgeOptimizer still 3 fails — the cnn CI{VOV, Nonneg} collision case, same as before. Will be fixed by the
constraint-mapping plumbing.
- Rest pass.
Big remaining work (rough order):
1. Translate vi in MOI.is_valid, MOI.delete, MOI.get, MOI.set, MOI.modify, MOI.supports and the callback / primal / dual attribute getters
2. Translate functions containing variable indices via MOI.Utilities.map_indices when crossing the boundary (ConstraintFunction,
ObjectiveFunction, VariablePrimal, change types, etc.)
3. Constraint mapping per (F, S) — activate on first VI/VOV force-bridge, translate at boundaries
4. Remove negative-index conventions
Core namespace machinery (src/Bridges/bridge_optimizer.jl):
- outer_to_inner(b, idx) / inner_to_outer(b, idx) — the canonical "if variable bridges are used → use the index map, else identity"
helpers, for both VariableIndex and ConstraintIndex{F,S}. CI{VariableIndex,S} translation is derived from the variable mapping (since
ci.value == vi.value by MOI convention), so it needs no stored entries.
- _OuterToInner / _InnerToOuter / _TotalInnerToOuter functor structs (<: Function so they work with map_indices). The "total" variant
leaves unrecorded indices unchanged — those are bridge-created inner variables later removed by unbridged_function.
- _to_inner_value / _from_inner_value — whole-value translation at the b.model boundary, applied via map_indices.
- _unbridged_result_from_inner / _unbridged_result_from_bridge — result processing that respects where a value came from:
recursive_model(b) === b (Lazy, outer namespace) vs b.model (SBO, inner namespace).
Key insight encoded in bridged_variable_function: for LazyBridgeOptimizer the substituted expressions stay outer (translation happens at
the model boundary); for SingleBridgeOptimizer they're already inner, so passthrough variables translate during substitution and the
recursion is skipped.
Plumbed sites: add_constraint(s), add_constrained_variable(s), delete (vi/ci/vectors, with map cleanup), is_valid,
variable/constraint/model attribute get/set, ObjectiveFunction get, modify (with _to_inner_change and _modify_substituted_change to avoid
double-translating decomposed modifications), VariableName/ConstraintName get/set, name→index lookups (including reverse-lookup of names
delegated to bridge-created variables), ListOfVariableIndices.
Activation triggers: variable mapping activates on first variable bridge; per-(F,S) constraint mapping activates on first force-bridged
VOV; add_bridged_constraint avoids identity-copied indices via _is_available_constraint_index.
One Utilities change: substitute_variables(map, x::VariableIndex) now allows index-to-index renaming, erroring only when bridged into a
real function.
Test updates (3 places where tests reached around the bridge layer to the mock with outer indices — they now use the inner index,
consistent with how they already handled y).
…StackOverflow is gone). Remaining failures, all test-only, two kinds: 1. Hardcoded old negative values (e.g. test_FreeBridge:138 expecting [-1,-2]) — trivially update to the new positive values. 2. Cross-layer is_valid isolation (test_nesting lines 648, 664–666: !is_valid(model, x), !is_valid(b0, cnn), etc.). These assert that a bridged entity from an upper layer is invalid in lower layers. With the old negatives this held for free; with positive translated indices a bridged variable's value (x=1) coincides with an inner-model variable, so a direct is_valid(model, x) is a false positive. Real operations are unaffected because they route through the proper layer — only these direct cross-namespace value comparisons see the coincidence. The new design's guarantee is "operations through the correct layer are correct," not "indices are globally unique across layers" (which is what the old negative encoding incidentally provided).
Progress: 20→10 lazy failures (PSD fixed). The remaining 10 are all in test_linear_integration with the restricted SDPA model — MOI.get(ObjectiveFunction) comes back with untranslated indices (VariableIndex(7,9) instead of the user's v1,v2) and a wrong constant. This is the objective-function unbridging/translation path for deeply-bridged Lazy models — another cross-namespace translation site, but a non-trivial one (involves the bridge's substitution chain, not just index relabeling). Let me take stock and give you a clear status, since this has become an extended whack-a-mole and you should decide the next move. Where things stand Done (committed by you through e2d8d2a + my newer uncommitted edits): - Full positive-index conversion: Variable/map.jl (slot bijection), bridge_optimizer.jl dispatch (is_bridged is now a map lookup), Constraint/map.jl (positive VOV). - Six collision-specific fixes, each from the outer-bridged-value == inner-created-value coincidence that negative indices used to prevent: a. unbridged_function infinite recursion (gated on recursive_model) b. _get_all_including_bridged chain-loop self-loop hang (gated on recursive_model) c. _remove_bridged wrongly emptying the variable list (no-op for ListOfVariableIndices) d. vector-member expansion value - i → value + i (ascending), 2 sites e. has_keys ordering check (slot-consecutive → ascending-value, allows post-deletion gaps) f. Map constructor next_outer_variable default 0 → 1 (self-contained) g. main-loop "raw inner first" lookup → always-translate (fixed the PSD duplicate) - Test updates: test_FreeBridge, test_NonposToNonneg, test_VectorizeBridge, test_map, test_nesting (dropped cross-layer is_valid isolation assertions that positive indices can't provide). Verified green: test_bridge_optimizer.jl (General) ✅, all 11 test/Bridges/Variable/ ✅. Remaining: lazy suite — 10 failures in test_linear_integration (objective unbridging for the restricted SDPA model). Constraint suite untested (needs the CliqueTrees test dep). Likely a few more cross-namespace sites lurk in the deeply-bridged Lazy paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here, we simply use an IndexMap to map inner and outer variables. This IndexMap is only created when variable bridges are used.
So if no variable bridges are used, this shouldn't affect performance or anything.
If variable bridges are used, it's already slow because we need to substitute variables in expressions so the IndexMap shouldn't be a bottleneck.
This doesn't require adding the reservation indices API like #3010 so it should be much better.
Closes #3010