Use formulations as an interface #59

josephmckinsey · 2025-02-11T02:40:28Z

Should solve #58 along with making everything more extensible.

This involved a lot of refactoring, some documentation, and more testing to make sure it works like the old version.

I also made some departures from how Sienna does it:

Formulations have data, not DeviceModels directly. This allows you to configure formulations more independently.
The device models in a problem template are actually a list. We traverse the list in reverse order and find the first formulation that applies to figure out which formulation to use for each component.

codecov · 2025-02-11T02:47:39Z

Codecov Report

Attention: Patch coverage is 85.15625% with 38 lines in your changes missing coverage. Please review.

Project coverage is 52.17%. Comparing base (90fd4dc) to head (d699663).

Files with missing lines	Patch %	Lines
src/PowerSystems2PRAS.jl	83.91%	32 Missing ⚠️
src/formulation_definitions.jl	88.23%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
+ Coverage   48.73%   52.17%   +3.44%     
==========================================
  Files          12       13       +1     
  Lines         788      851      +63     
==========================================
+ Hits          384      444      +60     
- Misses        404      407       +3

Flag	Coverage Δ
unittests	`52.17% <85.15%> (+3.44%)`	⬆️

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

Files with missing lines	Coverage Δ
src/SiennaPRASInterface.jl	`100.00% <100.00%> (ø)`
src/formulation_definitions.jl	`88.23% <88.23%> (ø)`
src/PowerSystems2PRAS.jl	`83.05% <83.91%> (+0.69%)`	⬆️

... and 2 files with indirect coverage changes

jd-lara · 2025-02-12T19:00:01Z

src/PowerSystems2PRAS.jl

-    # Check if any GeometricDistributionForcedOutage objects exist in the System
-    outages = PSY.get_supplemental_attributes(PSY.GeometricDistributionForcedOutage, sys)
+function add_default_data!(sys::PSY.System)
+    @warn "No forced outage data available in the Sienna/Data PowerSystems System. Using generic outage data ..."


Can take this out of this function and make something more clean where we pass the CSV file? I don't really like that we rely on a CONST file definition for this.

Are you saying we should let the user pass a CSV with default values? If that is the case, we might need to specify a set of necessary columns or something to map from custom column names to the columns the parser expects.

Ok, I made it a parameter with a default value of the const value. Do we want to make this function public or exported? I didn't do that yet.

It makes sense to make this function public. It is up to the user to format the CSV in a way we can ingest it. We might want to add column names and description of this CSV to documentation. Before we parse, we want to make sure the CSV has the necessary columns. Happy to discuss if you have any questions.

I think we could probably handle this in a separate PR. #64

lets not make CSV parser a default thing here. The data and models are separate. @scdhulipala if you want to make a data model to parse outages and put it in PSY that's what we need to do.

jd-lara · 2025-02-12T19:01:52Z

src/PowerSystems2PRAS.jl


+    # FIXME
    for (idx, region) in enumerate(regions)
        reg_load_comps =
            get_available_components_in_aggregation_topology(PSY.StaticLoad, sys, region)


Be aware that a system can contain other load types. This should probably support all subtypes of ElectricLoad

I did not plan on messing with load or lines yet #61

jd-lara · 2025-02-12T19:03:11Z

src/PowerSystems2PRAS.jl

@@ -396,33 +302,29 @@ function generate_pras_system(
    λ_gen = Matrix{Float64}(undef, n_gen, s2p_meta.N)
    μ_gen = Matrix{Float64}(undef, n_gen, s2p_meta.N)

+    #FIXME


@josephmckinsey is this pending? Do we need to add an issue or a comment to this?

Yes. This needs to be fixed but this logic works for now. We will implement the component map solution we discussed for this.

open an issue for this please

Tracked in #60

jd-lara · 2025-02-12T19:04:47Z

src/PowerSystems2PRAS.jl

@@ -434,12 +336,12 @@ function generate_pras_system(
                                PSY.get_time_series_multiple(
                                    g,
                                    s2p_meta.filter_func,
-                                    name="max_active_power",
+                                    name=component_to_formulation[g].max_active_power,


can use getter function for these fields?

jd-lara · 2025-02-12T19:06:00Z

src/PowerSystems2PRAS.jl

-                PSY.get_prime_mover_type(g) == PSY.PrimeMovers.PVe
-            )
-        )
+        if haskey(PSY.get_ext(g), "region_gens")
            reg_gens_DA = PSY.get_ext(g)["region_gens"]
            gen_cap_array[idx, :] =
                round.(


This loop gives me a headache to follow what's up. We should abstract this into a function because it impossible to check if the logic is reasonable. @daniel-thom you might have some insight on the use of these functions for time series here.

Same comment as above. We will not need this logic if we implement the component map solution for lumping.

This is a mix of loop for the weird indexing PRAS needs and the lumped renewable generation. Later, I think we should spit the indexing into its own logic somehow later.

jd-lara · 2025-02-12T19:10:32Z

src/PowerSystems2PRAS.jl

+Apply HydroEnergyReservoir Formulation to fill in a row of a PRAS Matrix.
+Views should be passed in for all arrays.
+"""
+function assign_to_gen_stor_matrices!(


The logic here is also a bit unclear. Needs more comments to understand why some operations are happening

I can add more comments.

Yeah, I just pretty much copy pasted here.

src/PowerSystems2PRAS.jl

jd-lara

Overall the design for the formulations looks ok to me. There are several methods that grab time series that definitely need to be put into separate methods.

There are some parts that also need more comments because I can't really review the logic without knowing what I am looking at.

scdhulipala · 2025-02-12T21:49:16Z

src/SiennaPRASInterface.jl

@@ -50,6 +50,15 @@ export add_csv_time_series!
 export add_csv_time_series_single_stage!
 export make_generator_outage_draws!

+export GeneratorFormulation


General comment for all the formulation names, to follow PSI convention, can we call these GeneratorResourceAdequacy, etc. Do we need to call it PRASProblemTemplate? RATemplate?

Changed to GeneratorPRAS but have a AbstractRAFormulation. The generator, generator storage, storage classification is very specific to PRAS.

src/SiennaPRASInterface.jl

scdhulipala · 2025-02-12T22:33:50Z

src/formulation_definitions.jl

+
+Objects in Sienna that behave like storage are mapped to storage in PRAS.
+"""
+struct StorageFormulation <: AbstractPRASFormulation end


Do we want to add carry over efficiency to the formulation because currently we just assume it is 1.0?

call this LossLess formulation

src/formulation_definitions.jl

scdhulipala · 2025-02-12T23:26:30Z

src/formulation_definitions.jl

+"""
+Check whether a DevicePRASModel applies to a given type
+"""
+function appliestodevice(::DevicePRASModel{D}, ::Type{T}) where {D, T}


I'm not sure I follow how this doesn't allow PSY.EnergyReservoirStorage to not have a GeneratorFormulation?

You can totally do that in this framework. This only tells you about the PSY.Device subtype D and has nothing about the formulation type itself. Unless we specify some relation between the type parameters in DevicePRASModel, then it will error when it tries to actually apply a model.

scdhulipala · 2025-02-12T23:36:52Z

src/PowerSystems2PRAS.jl

+    for (idx, region) in enumerate(regions)
+        reg_ren_comps = filter(
+            x -> haskey(component_to_formulation, x),
+            get_available_components_in_aggregation_topology(PSY.RenewableGen, sys, region),


Do we need to do this again get_available_components_in_aggregation_topology ? Or can we get all components from the Dict with this formulation and filter for this?

I'm not sure we have a function to group by aggregation topology from a dictionary yet. It might be appropriate, but I didn't want to get into that here.

scdhulipala · 2025-02-12T23:40:03Z

src/PowerSystems2PRAS.jl

+Apply HydroEnergyReservoir Formulation to fill in a row of a PRAS Matrix.
+Views should be passed in for all arrays.
+"""
+function assign_to_gen_stor_matrices!(


I can add more comments.

src/PowerSystems2PRAS.jl

Joseph McKinsey added 2 commits February 6, 2025 11:44

Refactor gen, genstor, etc into separate functions

33de2ba

Refactor to formulation interface

7be3a34

Joseph McKinsey added 4 commits February 11, 2025 13:23

Add new time series tests + renaming test

6d50e75

Improve documentation and testing

37110f7

Set up docs correctly

d706554

Use private=false instead for public documentation

40da93c

josephmckinsey changed the title ~~WIP: Use formulations as an interface~~ Use formulations as an interface Feb 11, 2025

josephmckinsey requested review from scdhulipala and jd-lara February 11, 2025 23:20

josephmckinsey marked this pull request as ready for review February 11, 2025 23:20

jd-lara reviewed Feb 12, 2025

View reviewed changes

src/PowerSystems2PRAS.jl Outdated Show resolved Hide resolved

jd-lara requested changes Feb 12, 2025

View reviewed changes

scdhulipala requested changes Feb 12, 2025

View reviewed changes

Joseph McKinsey added 10 commits February 13, 2025 11:34

Pass in default data filepath instead

01a6220

Add more detailed comment on FIXME for process generator

00be1ff

Switch to getters for formulations

a52de5d

Refactor floored time series into a function

00ee9ec

Change estimate to analyze in doc-strings

6344c2b

Add D <: PSY.Device

d5bb700

Make an EnergyReservoirLossless model

1240533

Rename to make things more generic.

d8a7a5d

Document and rename storage formulation to StoragePRAS

e813791

Fix storage assign function

2eb7d3c

ktehranchi mentioned this pull request Feb 14, 2025

Pointing to ActivePowerTimeseries for renewable gens #58

Open

Add more tests for RATemplate and fix subtyping on constructor

d699663

Use formulations as an interface #59

Are you sure you want to change the base?

Use formulations as an interface #59

Conversation

josephmckinsey commented Feb 11, 2025 • edited Loading

codecov bot commented Feb 11, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jd-lara left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephmckinsey commented Feb 11, 2025 •

edited

Loading

codecov bot commented Feb 11, 2025 •

edited

Loading