[hellemo]

Problem. inputs(l) / outputs(l) of a link is link_res(l), an intersect of the resources of the two connected nodes. It is evaluated far more often than needed while building a model:

• in the index sets of link_in[l ∈ ℒ, 𝒯, inputs(l)] and link_out[…], where JuMP evaluates the last set for every (l, t);
• in the coupling condition p ∈ inputs(l) of constraints_couple, for every (n, t, p, l);
• and link_sub(ℒ, n) filters all links for every node.

The node flows have the same pattern: inputs(n) / outputs(n) collect the keys of the node's resource Dict once per operational period.

Change (src/model.jl, +30/−11):

• variables_flow computes the resources of each node and each link once, as Dict(n => inputs(n) for n ∈ 𝒩ⁱⁿ) and so on, and uses these in the index sets and in the bound loops.
• constraints_couple computes the resources of each link and the links of each node once, in one pass over the links, keeping the order link_sub gives.

There are no API changes, and link_sub, inputs and outputs are unchanged.

Same model. For CaseBodo (EMB with EMI, EMRP, EMF and EMH), every build was compared with the unpatched one by variable names: the same variables, bounds and objective, and bitwise the same constraint rows as a multiset.

Speed. CaseBodo, warm create_model on a case set up once, string names off, minimum of 3 builds per run, 2–4 runs per variant with registry EMB 0.10.8 and patched EMB alternating; all other packages identical:

instance	variables	before	after	speedup
4 weeks, base year	476K	2.52–2.67 s	1.85–1.99 s	1.3–1.4×
4 weeks, 4 strategic periods	1.90M	12.9–14.0 s	9.8–10.8 s	1.2–1.4×
12 weeks, base year	1.43M	9.5–9.8 s	6.5–6.9 s	1.4–1.5×

Possible follow-up. Caching link_res in the link, or memoising it, would give the same benefit to every other caller of inputs(l) / outputs(l), for example in extension packages.

🤖 Generated with Claude Code

[JulStraus]

I am not necessarily opposed to the changes. Only filtering for the values once can speed up model construction quite a bit. We saw that as well in EnergyModelsRecedingHorizon.

There are a few things I would like to highlight/mention:

1. I personally think it is better to create new methods for inputs and outputs, i.e.,:

   inputs_dict(𝒩ⁱⁿ::Vector{<:Node} = Dict(n => inputs(n) for n ∈ 𝒩ⁱⁿ)
   outputs_dict(𝒩ᵒᵘᵗ::Vector{<:Node} = Dict(n => inputs(n) for n ∈ 𝒩ᵒᵘᵗ)

   and comparable for the links. The reason for my approach is that it can be utilized also in potentially other circumstances.

2. The same holds for link_sub. We can here even change the behavior as it is not exported nor utilized in any package. Moving it out of the loop would be, as usual, rather beneficial.

3. Caching the resources within a link is fine in my opinion. The function link_res is essentially a backup solution and should not necessarily be utilized in other packages. We use it as a backup in some functions although it is not really well utilized. An example is create_element(m, l::Link, 𝒯, 𝒫, modeltype::EnergyModel) where we should have utilized vcat(inputs(l), outputs(l)) to avoid problems with links with changing inputs and outputs. In my opinion, we could easily include that in an inner constructor for Direct without creating breaking changes, i.e.,

   struct Direct <: Link
       id::Any
       from::Node
       to::Node
       inputs::Vector{<:Resource}
       outputs::Vector{<:Resource}
       formulation::Formulation
       function Direct(id, from::Node, to::Node, formulation::Formulation)
           𝒫 = intersect(inputs(l.to), outputs(l.from))
           new(id, from, to, 𝒫, 𝒫, formulation)
       end
   end

   However, I would see that as a separate PR. The question remains how much of a speed advantage we do get. More for larger models, obviously, but the intersect operation which is used for inputs(l) and outputs(l) is normally quite fast as we only compare vectors of in maximum the number of resources.

Important

I think we have quite a few additional points where we can improve the speed, either through caching, or through slight modifications regarding how we write the code. We can see this PR as a beginning.

I did not approve yet, as I would like to see your thoughts regarding storing the information in sub functions instead.

Claude was however way to elaborate again in the PR text... Please tell Claude the next time that it should be concise. The PR message does not need equal amount of lines as changed in the code.

[hellemo]

Thanks, sorry about the verbose PR, my own tend to be a bit on the terse side. Some performance measurements to back the claim was the main motivation to use Claude's writeup.

I think adding helper functions makes sense for consistency across the package, but I also quite like the idea of using memoization. Wit memoization we could keep the current formulation/style and it would be cached under the hood, also available for any extension without modification.

The main downside I see for that approach is that we either need to implement it in EMB directly or add a dependency on one of the memoization packages.

Like you say, there are more opportunities for optimizations, but this showed up in my profiling,

[JulStraus]

Thanks, sorry about the verbose PR, my own tend to be a bit on the terse side. Some performance measurements to back the claim was the main motivation to use Claude's writeup.

I think adding helper functions makes sense for consistency across the package, but I also quite like the idea of using memoization. Wit memoization we could keep the current formulation/style and it would be cached under the hood, also available for any extension without modification.

The main downside I see for that approach is that we either need to implement it in EMB directly or add a dependency on one of the memoization packages.

Like you say, there are more opportunities for optimizations, but this showed up in my profiling,

I think we can test out memoization. Does it require additional dependencies?