Add add_device_terms_to_expression! driver for device-injection methods#112
Open
luke-kiernan wants to merge 2 commits into
Open
Add add_device_terms_to_expression! driver for device-injection methods#112luke-kiernan wants to merge 2 commits into
luke-kiernan wants to merge 2 commits into
Conversation
Contributor
|
Performance Results This branch |
Generic driver that applies a per-(device, time) proportional term to one or two balance-expression targets, separating the network-model axis (targets_fn) from the variable/parameter/constant source (term_fn). This lets downstream add_to_expression! methods share a single device/time loop instead of re-implementing it. Stays type-stable / allocation-free on heterogeneous target tuples (e.g. AreaPTDF's String-keyed area expression plus Int-keyed nodal expression) via Base.tail recursion over the statically-sized targets tuple. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
luke-kiernan
force-pushed
the
lk/add-device-terms-driver
branch
from
0ca9dc3 to
42ccd82
Compare
luke-kiernan
changed the base branch from
ac/canonical-key-component-type
to
main
Collaborator
Author
|
I intend to add tests, but I'm waiting until I see if Jose is on board with how I've refactored things in the companion POM PR. I'm using closures, which we usually try to avoid. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new exported helper API to factor out the common device × time looping pattern used by downstream “device injection” add_to_expression! implementations, centralizing the looping logic in IOM while letting downstream code provide target selection and per-device term generation.
Changes:
- Export
add_device_terms_to_expression!from the main module. - Add
add_device_terms_to_expression!generic driver to apply per-device/per-time proportional terms to 1–2 expression targets. - Add internal
_apply_term_to_targets!helper using tuple tail recursion to keep heterogeneous target tuples type-stable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/InfrastructureOptimizationModels.jl |
Exports the new public driver function. |
src/common_models/add_jump_expressions.jl |
Implements the new driver and its internal target-application helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+100
to
+101
| # nodal entry plus a system/area entry). Tail recursion ensures type stability for | ||
| # the hetereogeneous length 2 tuples. |
Comment on lines
+102
to
+110
| const _BalanceTermValue = Union{Float64, JuMP.AbstractJuMPScalar} | ||
|
|
||
| _apply_term_to_targets!(::Tuple{}, ::_BalanceTermValue, ::Float64, ::Int) = nothing | ||
|
|
||
| function _apply_term_to_targets!( | ||
| targets::Tuple, | ||
| value::_BalanceTermValue, | ||
| multiplier::Float64, | ||
| t::Int, |
Member
|
Lets merge this once POM is cleared up from the pending work |
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
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.
What
Adds
add_device_terms_to_expression!— a generic driver for device-injectionadd_to_expression!methods — plus its internal_apply_term_to_targets!helper, and exports the driver.Why
Downstream (POM) has a large family of
add_to_expression!methods that are the samedevice × timeloop differing only in (a) which expression entries a device contributes to (network-model dependent) and (b) the variable/parameter/constant term. This driver factors out the loop, so those methods collapse to a small target resolver + a term closure. The POM-side consolidation lands separately (POM #135).Design
targets_fn(d)returns a 1- or 2-element tuple of(expression_matrix, row_index)targets (one for single-bus/area/system models; two for PTDF/AreaPTDF).term_fn(d)returns a per-devicet -> (value, multiplier)closure._apply_term_to_targets!consumes the targets viaBase.tailrecursion rather than aforloop: this stays type-stable and allocation-free even when the tuple is heterogeneous (AreaPTDF mixes aString-keyed area expression and anInt-keyed nodal expression, which aforloop would box). Verified identical native codegen and 0 allocations.Target branch
Into
main. (Additive — the two touched files are identical betweenmainandac/canonical-key-component-type, so it applies cleanly to either.)🤖 Generated with Claude Code