Create ComponentPairKey for service models - #171
Conversation
|
Performance Results This branch |
|
Can you make a minimum example that showcases how our approach was failing before |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate findings remain in variable creation, key encoding, and tolerance dispatch tests.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
Adds ComponentPairKey so service variables are keyed by device and service types, avoiding naming conflicts.
Changes:
- Adds and exports composite key handling.
- Refactors service-variable containers by device/service pairs.
- Updates mocks and related tests.
| File | Summary |
|---|---|
test/verify_mocks.jl |
Updates mock construction. |
test/test_tolerance_dispatch.jl |
Moderate (3): Refixed variables need force for each sample at lines 95 and 149. |
test/test_quadratic_approximations.jl |
Removes an unused alias. |
test/test_optimization_container_keys.jl |
Tests composite keys and service-variable separation. |
test/mocks/mock_services.jl |
Adds parameterized service mocks. |
src/InfrastructureOptimizationModels.jl |
Exports ComponentPairKey. |
src/core/optimization_container_keys.jl |
Critical (1): Pair encoding can collide with generic parametric component encoding. |
src/common_models/add_variable.jl |
Critical (1): Inserts the Vector type instead of an empty vector. Moderate (1): Unconditionally creates containers instead of reusing existing ones. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| JuMP.@objective(setup.jump_model, Min, expr["g", 1]) | ||
| JuMP.set_optimizer(setup.jump_model, HiGHS.Optimizer) | ||
| JuMP.set_silent(setup.jump_model) | ||
| JuMP.fix(x, x0) |
|
@luke-kiernan Does generating a container for each component-type service-type fix the stability issue we had before when we were looping through different StaticInjection. I don't remember if this can fix the stability issue. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Service model variables need to be keyed on the service type and device type due to possible naming conflicts.