make branch filters and interfaces compatible

[m-bossart]

Fixes #266

[github-actions]

Performance Results

Version	Precompile Time
Main	6.752177519
This Branch	6.694894322

Version	Build Time
Main-Build Time Precompile	105.793617442
Main-Build Time Postcompile	6.495289466
This Branch-Build Time Precompile	100.507937026
This Branch-Build Time Postcompile	6.21297872

Version	Solve Time
Main-Solve Time Precompile	128.646602594
Main-Solve Time Postcompile	97.600927765
This Branch-Solve Time Precompile	346.08595607
This Branch-Solve Time Postcompile	291.80615488

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[luke-kiernan]

I'm finding the logic here hard to follow: it seems like a tripping hazard for users and future devs. At the very least, I'd suggest making this "widening" opt-in instead of on-by-default.

Another thought: could you get the same results merely by re-writing your filter function? I haven't dug into the details of services, but basically

new_filter(x) = old_filter(x) || x in [something]

[luke-kiernan]

typeof(branch) -> type parameter

[luke-kiernan]

Some type instability here: values(service_models) looks like it's heterogeneous, typeof(service). Might be worth asking AI how much can be done about that

[luke-kiernan]

Read from the system's own contributing-device mapping
rather than from the service model's map, which has already been narrowed by component type
and so cannot report a branch type the template fails to model.

Well that's rather confusing

[luke-kiernan]

Do we have test cases for these?

[luke-kiernan]

pre-existing perf nitpick: make branch_keys_to_delete a set and then do set difference

[m-bossart]

I'm finding the logic here hard to follow: it seems like a tripping hazard for users and future devs. At the very least, I'd suggest making this "widening" opt-in instead of on-by-default.

Another thought: could you get the same results merely by re-writing your filter function? I haven't dug into the details of services, but basically

new_filter(x) = old_filter(x) || x in [something]

You could get the same behavior from a filter function, but this makes it harder for the user to write a simple filter function. This PR is just encoding the assumption that if I am modeling an interface that includes branches, those branches must be modeled. Note that we still error if the user has decided not to model a type at all that is in an interface. In that case, the user needs to make a deliberate change (to add the device or remove the interface).

[m-bossart]

I'm finding the logic here hard to follow: it seems like a tripping hazard for users and future devs. At the very least, I'd suggest making this "widening" opt-in instead of on-by-default.
Another thought: could you get the same results merely by re-writing your filter function? I haven't dug into the details of services, but basically

new_filter(x) = old_filter(x) || x in [something]

You could get the same behavior from a filter function, but this makes it harder for the user to write a simple filter function. This PR is just encoding the assumption that if I am modeling an interface that includes branches, those branches must be modeled. Note that we still error if the user has decided not to model a type at all that is in an interface. In that case, the user needs to make a deliberate change (to add the device or remove the interface).

@jd-lara Do you have a preference of if we widen the branch filter automatically to include branches that appear in services, or error and require the user to include those branches by modifying the user provided filter function?

If we do just rely on the filter function, we still need to make the change to error (right now the answer is silently wrong)

[jd-lara]

I'm finding the logic here hard to follow: it seems like a tripping hazard for users and future devs. At the very least, I'd suggest making this "widening" opt-in instead of on-by-default.
Another thought: could you get the same results merely by re-writing your filter function? I haven't dug into the details of services, but basically

new_filter(x) = old_filter(x) || x in [something]

You could get the same behavior from a filter function, but this makes it harder for the user to write a simple filter function. This PR is just encoding the assumption that if I am modeling an interface that includes branches, those branches must be modeled. Note that we still error if the user has decided not to model a type at all that is in an interface. In that case, the user needs to make a deliberate change (to add the device or remove the interface).

@jd-lara Do you have a preference of if we widen the branch filter automatically to include branches that appear in services, or error and require the user to include those branches by modifying the user provided filter function?

If we do just rely on the filter function, we still need to make the change to error (right now the answer is silently wrong)

We only have a service the transmission interface, we should catch the user setting a filter and the interface and do the right thing otherwise it can be a mess with 100s of GTCs

[jd-lara]

@jarry7 any thoughts on this in effect always including the lines in the interfaces regardless of the filter. We either need to fail loudly that the filter is inconsistent with the interfaces or just handle it under the hood.

[luke-kiernan]

Articulating my concerns more precisely: I don't like how the filters change mid-process. Innoncent refactoring--e.g. caching the items that pass a filter--suddenly now can cause major changes in behavior. Going forward, if I have a function that uses filters, I need to think: where does it belong relative to the "widening" step? imo that's bad, a recipe for headaches and bugs.

Alternatives: maintain a collection of "extra things we also need to model" somewhere?

edit: assigning the modified filter to a different kwarg (add, don't overwrite) would address my objections on paper...but having 2 things that are almost but not quite the same is also bad. Then you need to keep track of which to use where.

[jd-lara]

Articulating my concerns more precisely: I don't like how the filters change mid-process. Innoncent refactoring--e.g. caching the items that pass a filter--suddenly now can cause major changes in behavior. Going forward, if I have a function that uses filters, I need to think: where does it belong relative to the "widening" step? imo that's bad, a recipe for headaches and bugs.

Alternatives: maintain a collection of "extra things we also need to model" somewhere?

edit: assigning the modified filter to a different kwarg (add, don't overwrite) would address my objections on paper...but having 2 things that are almost but not quite the same is also bad. Then you need to keep track of which to use where.

The true solution for this is to implement something like this

#279

So we can compose filtering adequately instead than with an arbitrary function

[luke-kiernan]

Due to my work in PNM, it occurred to me to ask: how do NetworkInterfaces, network reductions, and filters interact? There's been bugs in PNM with the 2nd and 3rd, bugs in POM with the 1st and the 3rd, so it stands to reason there might be more bugs in POM where all 3 overlap.

I asked Claude about this: not exactly a bug, but certianly confusing.

POM now has two different policies for the same underlying situation:

• Filtered-out contributor → this PR silently widens the filter to include it.
• Reduction-merged contributor with partial membership → existing code throws and tells the user to fix their data.

Both are "the interface names a branch the model doesn't carry a flow for." One is auto-repaired, the other is a hard error demanding data changes. A user who hits both gets opposite advice from the same system, and neither path mentions the other exists.

[m-bossart]

Due to my work in PNM, it occurred to me to ask: how do NetworkInterfaces, network reductions, and filters interact? There's been bugs in PNM with the 2nd and 3rd, bugs in POM with the 1st and the 3rd, so it stands to reason there might be more bugs in POM where all 3 overlap.

I asked Claude about this: not exactly a bug, but certianly confusing.

POM now has two different policies for the same underlying situation:

• Filtered-out contributor → this PR silently widens the filter to include it.
• Reduction-merged contributor with partial membership → existing code throws and tells the user to fix their data.

Both are "the interface names a branch the model doesn't carry a flow for." One is auto-repaired, the other is a hard error demanding data changes. A user who hits both gets opposite advice from the same system, and neither path mentions the other exists.

I think the answer here is to pin branches in the interface from being reduced. This is already the policy for DLRs, controllable transformers, etc. It is the same idea, if the template needs to include something to model it correctlyh, it should be pinned from reductions. This is orthogonal to the filtering question. My suggestion is this:
Implement Now:

• Error on filters and interfaces being incompatible, instead of being silently wrong. Require the user to define a filter function compatible with interface definitions if they want both.
• Pin buses for branches in interfaces (@luke-kiernan comment above).

Open issue and Implement later:

• A more robust solution based on ComponentSelector alleviates the concerns above about modifying the filter.

[m-bossart]

Also relevant to this discussion is handling monitored components of a contingency. This can fail currently if a monitored component is filtered about by the user provided filter.