Add `ComponentSelector` feature: IS portion #342

GabrielKS · 2024-03-18T23:51:43Z

~~This draft PR will get filled in as @tengis-nrl and I move the ComponentSelector features from PowerAnalytics to InfrastructureSystems.~~ Ready for review! Adds an immutable, lazy, system-independent representation of a grouped set of components. For more details and a demonstration, see https://github.nrel.gov/gkonars/PowerAnalytics-demos/blob/main/component_selector_pr_demo.ipynb.

src/component_selector.jl

codecov · 2024-03-24T07:21:22Z

Codecov Report

Attention: Patch coverage is 89.84375% with 13 lines in your changes missing coverage. Please review.

Project coverage is 78.47%. Comparing base (e443a83) to head (5bfccd4).
Report is 60 commits behind head on main.

Files with missing lines	Patch %	Lines
src/results.jl	0.00%	9 Missing ⚠️
src/time_series_storage.jl	0.00%	2 Missing ⚠️
src/component_container.jl	92.85%	1 Missing ⚠️
src/component_selector.jl	98.96%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #342      +/-   ##
==========================================
+ Coverage   77.94%   78.47%   +0.53%     
==========================================
  Files          69       71       +2     
  Lines        5622     5743     +121     
==========================================
+ Hits         4382     4507     +125     
+ Misses       1240     1236       -4

Flag	Coverage Δ
unittests	`78.47% <89.84%> (+0.53%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/InfrastructureSystems.jl	`55.55% <ø> (ø)`
src/Optimization/optimization_container_keys.jl	`95.58% <100.00%> (ø)`
src/common.jl	`66.66% <ø> (ø)`
src/components.jl	`86.39% <ø> (ø)`
src/system_data.jl	`92.01% <100.00%> (+0.01%)`	⬆️
src/component_container.jl	`92.85% <92.85%> (ø)`
src/component_selector.jl	`98.96% <98.96%> (ø)`
src/time_series_storage.jl	`61.53% <0.00%> (ø)`
src/results.jl	`0.00% <0.00%> (ø)`

... and 2 files with indirect coverage changes

tengis-nrl · 2024-03-25T16:41:30Z

jl_5D19.tmp

GabrielKS · 2024-03-27T15:25:12Z

We need working implementations of get_components and get_subselectors here, not just in PowerSystems, because the ComponentSelector feature is meant to be usable without PowerSystems. You'll note a TODO here about making get_available optional; if you don't think we have time to do that now, I think the IS version of get_components should just not check availability and the PSY version should.

GabrielKS · 2024-06-10T18:46:15Z

Rebased onto main and tests pass.

GabrielKS · 2024-09-26T00:30:26Z

This is now ready for in-depth code review! For a demonstration, see https://github.nrel.gov/gkonars/PowerAnalytics-demos/blob/main/component_selector_pr_demo.ipynb.

daniel-thom

This is a first pass to get some pedantic stuff out of the way.

src/component_selector.jl

src/common.jl

src/component_selector.jl

daniel-thom · 2024-09-30T19:34:22Z

src/component_selector.jl

+function get_components(e::FilterComponentSelector, sys::SYSTEM_LIKE; filterby = nothing)
+    components = get_components(e.filter_fn, e.component_subtype, sys)
+    isnothing(filterby) && (return components)
+    return Iterators.filter(filterby, components)


This makes the function type unstable. We created FlattenIteratorWrapper so that we know the length. We shouldn't return different types based on a kwarg.

Are there any performance concerns with adding the second filter + iterator? Basically, how much slower is this implementation vs combining the two filter functions into one?

Agreed, I'll make these type stable.

Looks like not really, not after compilation anyway:

(this is on the massive EI system)

get_components now always returns FlattenIteratorWrapper. I also ended up combining the two filter functions into one as an implementation detail (not for performance, it just ended up being more convenient with FlattenIteratorWrapper).

This reverts commit 15b3f31.

daniel-thom

LGTM

github-actions bot reviewed Mar 24, 2024

View reviewed changes

src/component_selector.jl Outdated Show resolved Hide resolved

src/component_selector.jl Outdated Show resolved Hide resolved

src/component_selector.jl Show resolved Hide resolved

tengis-nrl marked this pull request as ready for review March 25, 2024 16:22

jd-lara reviewed Mar 26, 2024

View reviewed changes

jl_5D19.tmp Outdated Show resolved Hide resolved

GabrielKS assigned tengis-nrl Mar 27, 2024

jd-lara assigned jd-lara and unassigned tengis-nrl Apr 22, 2024

GabrielKS changed the base branch from gks/td/component_selector_feature to main June 4, 2024 17:53

GabrielKS force-pushed the gks/td/component_selector_port branch from 1732638 to 83ea408 Compare June 10, 2024 18:44

GabrielKS assigned GabrielKS and unassigned jd-lara Jun 10, 2024

GabrielKS requested a review from daniel-thom June 10, 2024 23:50

GabrielKS mentioned this pull request Jun 11, 2024

Move ComponentSelector from PowerAnalytics to InfrastructureSystems: PA portion GabrielKS/PowerAnalytics.jl#2

Merged

GabrielKS force-pushed the gks/td/component_selector_port branch from f4e395d to d4c3aff Compare June 26, 2024 21:43

GabrielKS force-pushed the gks/td/component_selector_port branch 2 times, most recently from 0344a74 to 915d247 Compare July 17, 2024 21:08

GabrielKS force-pushed the gks/td/component_selector_port branch from 915d247 to b11ccf6 Compare September 18, 2024 20:49

GabrielKS mentioned this pull request Sep 18, 2024

Add ComponentSelector feature: PSY portion (take two) NREL-Sienna/PowerSystems.jl#1197

Merged

GabrielKS force-pushed the gks/td/component_selector_port branch from c5de532 to 78acc9d Compare September 25, 2024 22:35

GabrielKS changed the title ~~Move ComponentSelector from PowerAnalytics to InfrastructureSystems: IS portion~~ Add ComponentSelector feature: IS portion Sep 26, 2024

GabrielKS mentioned this pull request Sep 26, 2024

Standardize Importing Conventions From IS #388

Open

GabrielKS requested a review from jd-lara September 26, 2024 00:30

daniel-thom reviewed Sep 28, 2024

View reviewed changes

daniel-thom reviewed Sep 30, 2024

View reviewed changes

src/component_selector.jl Outdated Show resolved Hide resolved

src/component_selector.jl Outdated Show resolved Hide resolved

src/component_selector.jl Outdated Show resolved Hide resolved

daniel-thom reviewed Sep 30, 2024

View reviewed changes

GabrielKS mentioned this pull request Oct 1, 2024

Feature: allow the use of ComponentSelector to filter devices to model NREL-Sienna/PowerSimulations.jl#1152

Open

GabrielKS added 23 commits January 17, 2025 13:15

ComponentSelector: Change typing to accommodate PowerSystems

fdc6b24

Address PR comments: rename subtype -> type, etc.

64d29cb

Address PR comments: add function-first interfaces

f378055

Address PR comments: rename filterby -> scope_limiter, etc.

fcbeaf4

ComponentSelector: validate in constructors, eliminate default_name

af4753d

Make get_components always return FlattenIteratorWrapper

57cdaf9

Implement filter(..., ::IS.FlattenIteratorWrapper)

6cbd928

Revert "Implement filter(..., ::IS.FlattenIteratorWrapper)"

ee78d4b

This reverts commit 15b3f31.

ComponentSelector: Eliminate scope_limiter from public interface

adc09a9

Require filter function as first argument in make_selector

e6cc155

Make groupby default to :each instead of :all

33189da

Implement rebuild_selector: easy cases

993206b

Add RegroupedComponentSelector to regroup ListComponentSelector

8b0ec63

Add get_available... methods in IS

984d6b2

Add SystemLike abstract type

abe3388

Rename SystemData to ComponentContainer

5f5e871

Centralize implementations of 'availability' in IS

74445d4

Implement get_component(s) for IS.Results

866a965

Add InfrastructureSystemsContainer methods on ComponentContainer

18fb728

ComponentSelector: Clean up default name code

924ef4e

Remove duplicate get_component(s) definitions

1967b1f

Move ComponentContainer tests to their own file

0df7915

ComponentSelector: make scope_limiter an optional first argument

701591e

GabrielKS force-pushed the gks/td/component_selector_port branch from edaf6b1 to 701591e Compare January 17, 2025 20:16

Fix typo, remove signatures from docstrings

5bfccd4

GabrielKS requested a review from daniel-thom January 17, 2025 22:09

daniel-thom approved these changes Jan 18, 2025

View reviewed changes

jd-lara approved these changes Jan 18, 2025

View reviewed changes

jd-lara merged commit 4807dca into main Jan 18, 2025
10 checks passed

jd-lara deleted the gks/td/component_selector_port branch January 19, 2025 06:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ComponentSelector` feature: IS portion #342

Add `ComponentSelector` feature: IS portion #342

GabrielKS commented Mar 18, 2024 •

edited

Loading

codecov bot commented Mar 24, 2024 •

edited

Loading

tengis-nrl commented Mar 25, 2024

GabrielKS commented Mar 27, 2024

GabrielKS commented Jun 10, 2024

GabrielKS commented Sep 26, 2024

daniel-thom left a comment

daniel-thom Sep 30, 2024

daniel-thom Sep 30, 2024

GabrielKS Sep 30, 2024

GabrielKS Oct 4, 2024

daniel-thom left a comment

Add ComponentSelector feature: IS portion #342

Add ComponentSelector feature: IS portion #342

Conversation

GabrielKS commented Mar 18, 2024 • edited Loading

codecov bot commented Mar 24, 2024 • edited Loading

Codecov Report

tengis-nrl commented Mar 25, 2024

GabrielKS commented Mar 27, 2024

GabrielKS commented Jun 10, 2024

GabrielKS commented Sep 26, 2024

daniel-thom left a comment

Choose a reason for hiding this comment

daniel-thom Sep 30, 2024

Choose a reason for hiding this comment

daniel-thom Sep 30, 2024

Choose a reason for hiding this comment

GabrielKS Sep 30, 2024

Choose a reason for hiding this comment

GabrielKS Oct 4, 2024

Choose a reason for hiding this comment

daniel-thom left a comment

Choose a reason for hiding this comment

Add `ComponentSelector` feature: IS portion #342

Add `ComponentSelector` feature: IS portion #342

GabrielKS commented Mar 18, 2024 •

edited

Loading

codecov bot commented Mar 24, 2024 •

edited

Loading