Remove old dependency and TODOs, fix some power flow testing

Project.toml

@@ -7,7 +7,6 @@ version = "0.7.0"
 DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
 DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
-DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
 InfrastructureSystems = "2cd47ed4-ca9b-11e9-27f2-ab636a7671f1"
 JSON3 = "0f8b85d8-7281-11e9-16c2-39a750bddbf1"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
@@ -21,7 +20,6 @@ SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
 DataFrames = "1"
 DataStructures = "0.18"
 Dates = "1"
-DelimitedFiles = "1"
 InfrastructureSystems = "2"
 JSON3 = "1"
 LinearAlgebra = "1"

src/PowerFlows.jl

@@ -45,7 +45,6 @@ import DataFrames: DataFrame
 import Dates
 import DataStructures
 import DataStructures: OrderedDict
-import DelimitedFiles
 include("psse_exporter/support_tools.jl")
 
 end

src/psse_export.jl

@@ -559,7 +559,7 @@ function _write_raw(
         isfinite(QT) || (QT = PSSE_DEFAULT)  # Catch Inf, etc.
         QB = reactive_power_limits.min
         isfinite(QB) || (QB = PSSE_DEFAULT)
-        VS = PSY.get_magnitude(PSY.get_bus(generator))  # TODO is this correct? Should this be `get_internal_voltage` for `PSY.Source`?
+        VS = PSY.get_magnitude(PSY.get_bus(generator))
         IREG = get(PSY.get_ext(generator), "IREG", PSSE_DEFAULT)
         MBASE = PSY.get_base_power(generator)
         ZR, ZX = PSSE_DEFAULT, PSSE_DEFAULT

test/test_powerflow_data.jl

@@ -32,9 +32,7 @@ end
     sys_null_updated = deepcopy(sys_original)
     PF.update_system!(sys_null_updated, data_original)
     data_null_updated = PowerFlowData(ACPowerFlow(), sys_null_updated)
-    # TODO fix bug in `_reactive_power_redistribution_pv`, see https://github.com/NREL-Sienna/PowerFlows.jl/issues/44
-    @test IS.compare_values(powerflow_match_fn, data_null_updated, data_original;
-        exclude = Set([:bus_reactivepower_injection]))
+    @test IS.compare_values(powerflow_match_fn, data_null_updated, data_original)
 
     # Modified versions should not be the same as unmodified versions
     @test !@test_logs((:error, r"values do not match"),
@@ -54,7 +52,5 @@ end
     # The big one: update_system! with modified PowerFlowData should result in sys_modified
     sys_modify_updated = deepcopy(sys_original)
     PF.update_system!(sys_modify_updated, data_modified)
-    # TODO fix bug in `_reactive_power_redistribution_pv`, see https://github.com/NREL-Sienna/PowerFlows.jl/issues/44
-    @test IS.compare_values(powerflow_match_fn, sys_modify_updated, sys_modified;
-        exclude = Set([:reactive_power]))
+    @test IS.compare_values(powerflow_match_fn, sys_modify_updated, sys_modified)
 end

test/test_psse_export.jl

@@ -1,7 +1,6 @@
 test_psse_export_dir = joinpath(TEST_FILES_DIR, "test_psse_exports")  # at some point could move this to temp files
 isdir(test_psse_export_dir) && rm(test_psse_export_dir; recursive = true)
 
-# TODO second macro I've ever written, probably wants a refactor
 function _log_assert(result, msg, comparison_name)
     result ||
         @error "Failed check: $(string(msg))$(isnothing(comparison_name) ? "" :  " ($comparison_name)")"
@@ -268,15 +267,18 @@ function compare_systems_wrapper(sys1::System, sys2::System, sys2_metadata = not
     return first_result && second_result
 end
 
-function test_power_flow(sys1::System, sys2::System)
+function test_power_flow(sys1::System, sys2::System; exclude_reactive_flow = false)
     result1 = solve_powerflow(ACPowerFlow(), sys1)
     result2 = solve_powerflow(ACPowerFlow(), sys2)
-    @test compare_df_within_tolerance(result1["bus_results"],
+    reactive_power_tol =
+        exclude_reactive_flow ? nothing : POWERFLOW_COMPARISON_TOLERANCE
+    @test compare_df_within_tolerance("bus_results", result1["bus_results"],
         result2["bus_results"], POWERFLOW_COMPARISON_TOLERANCE)
-    @test compare_df_within_tolerance(
+    @test compare_df_within_tolerance("flow_results",
         sort(result1["flow_results"], names(result1["flow_results"])[2:end]),
         sort(result2["flow_results"], names(result2["flow_results"])[2:end]),
-        POWERFLOW_COMPARISON_TOLERANCE; line_name = nothing)
+        POWERFLOW_COMPARISON_TOLERANCE; line_name = nothing, Q_to_from = reactive_power_tol,
+        Q_from_to = reactive_power_tol, Q_losses = reactive_power_tol)
 end
 
 function read_system_and_metadata(raw_path, metadata_path)
@@ -295,6 +297,7 @@ function test_psse_round_trip(
     year::Int,
     export_location::AbstractString;
     do_power_flow_test = true,
+    exclude_reactive_flow = false,
 )
     raw_path, metadata_path = get_psse_export_paths(scenario_name, year, export_location)
     @test !isfile(raw_path)
@@ -306,7 +309,8 @@ function test_psse_round_trip(
 
     sys2, sys2_metadata = read_system_and_metadata(raw_path, metadata_path)
     @test compare_systems_wrapper(sys, sys2, sys2_metadata)
-    do_power_flow_test && test_power_flow(sys, sys2)
+    do_power_flow_test &&
+        test_power_flow(sys, sys2; exclude_reactive_flow = exclude_reactive_flow)
 end
 
 "Test that the two raw files are exactly identical and the two metadata files parse to identical JSON"
@@ -367,7 +371,7 @@ end
     exporter = PSSEExporter(sys, :v33)
     export_location = joinpath(test_psse_export_dir, "v33", "system_240")
     test_psse_round_trip(sys, exporter, "basic", 2024, export_location;
-        do_power_flow_test = false)  # TODO why is AC power flow not converging for reimport here?
+        exclude_reactive_flow = true)  # TODO why is reactive flow not matching?
 
     # Exporting the exact same thing again should result in the exact same files
     write_export(exporter, "basic2", 2024, export_location)
@@ -397,11 +401,11 @@ end
     @test_logs((:error, r"Mismatch on rate"), (:error, r"values do not match"),
         match_mode = :any, min_level = Logging.Error,
         compare_systems_wrapper(sys, reread_sys2, sys2_metadata))
-    # test_power_flow(sys2, reread_sys2)  # TODO why is power flow broken?
+    test_power_flow(sys2, reread_sys2; exclude_reactive_flow = true)  # TODO why is reactive flow not matching?
 end
 
 @testset "PSSE Exporter with RTS_GMLC_DA_sys, v33" begin
-    sys = build_system(PSISystems, "RTS_GMLC_DA_sys")
+    sys = create_pf_friendly_rts_gmlc()
     set_units_base_system!(sys, UnitSystem.SYSTEM_BASE)
 
     # PSS/E version must be one of the supported ones
@@ -411,7 +415,7 @@ end
     exporter = PSSEExporter(sys, :v33)
     export_location = joinpath(test_psse_export_dir, "v33", "rts_gmlc")
     test_psse_round_trip(sys, exporter, "basic", 2024, export_location;
-        do_power_flow_test = false)  # TODO why is AC power flow not converging for reimport here?
+        exclude_reactive_flow = true)  # TODO why is reactive flow not matching?
 
     # Exporting the exact same thing again should result in the exact same files
     write_export(exporter, "basic2", 2024, export_location)
@@ -442,7 +446,7 @@ end
         (:error, r"Mismatch on Vm"), (:error, r"Mismatch on θ"),
         match_mode = :any, min_level = Logging.Error,
         compare_systems_wrapper(sys, reread_sys2, sys2_metadata))
-    # test_power_flow(sys2, reread_sys2)  # TODO fix power flow, see above
+    test_power_flow(sys2, reread_sys2; exclude_reactive_flow = true)  # TODO why is reactive flow not matching?
 
     # Updating with changed value should result in a different reimport (PowerFlowData version)
     exporter = PSSEExporter(sys, :v33)
@@ -453,20 +457,19 @@ end
     update_exporter!(exporter, pf2)
     write_export(exporter, "basic5", 2024, export_location)
     reread_sys3, sys3_metadata = read_system_and_metadata("basic5", 2024, export_location)
-    # TODO fix bug in `_reactive_power_redistribution_pv`, see https://github.com/NREL-Sienna/PowerFlows.jl/issues/44
     @test compare_systems_wrapper(sys2, reread_sys3, sys3_metadata;
-        exclude_reactive_power = true)
+        exclude_reactive_power = true)  # TODO why is reactive power not matching?
     @test_logs((:error, r"values do not match"),
         (:error, r"Mismatch on active_power"), (:error, r"Mismatch on reactive_power"),
         (:error, r"Mismatch on Vm"), (:error, r"Mismatch on θ"),
         match_mode = :any, min_level = Logging.Error,
         compare_systems_wrapper(sys, reread_sys3, sys3_metadata))
-    # test_power_flow(sys3, reread_sys3)  # TODO fix power flow, see above
+    test_power_flow(sys2, reread_sys3; exclude_reactive_flow = true)  # TODO why is reactive flow not matching?
 
     # Exporting with write_comments should be comparable to original system
     exporter = PSSEExporter(sys, :v33; write_comments = true)
     test_psse_round_trip(sys, exporter, "basic6", 2024, export_location;
-        do_power_flow_test = false)
+        exclude_reactive_flow = true)  # TODO why is reactive flow not matching?
 end
 
 @testset "Test exporter helper functions" begin

test/test_utils/common.jl

@@ -9,16 +9,34 @@ powerflow_match_fn(
 powerflow_match_fn(a, b) = IS.isequivalent(a, b)
 
 # TODO temporary hack, see https://github.com/NREL-Sienna/PowerFlows.jl/issues/39
-function PowerSystems.get_reactive_power_limits(gen::RenewableNonDispatch)
-    gen_pf = get_power_factor(gen)
-    gen_q = get_max_active_power(gen) * sqrt((1 / gen_pf^2) - 1)
-    return (min = 0.0, max = gen_q)
-end
+PowerSystems.get_reactive_power_limits(::RenewableNonDispatch) = (min = 0.0, max = 0.0)
+
+# TODO another temporary hack
+"Create a version of the RTS_GMLC system that plays nice with the current implementation of AC power flow"
+function create_pf_friendly_rts_gmlc()
+    sys = build_system(PSISystems, "RTS_GMLC_DA_sys")
+    remove_component!(sys, only(get_components(TwoTerminalHVDCLine, sys)))  # HVDC power flow not implemented yet
+    # Modify some things so reactive power redistribution succeeds
+    for (component_type, component_name, new_limits) in [
+        (RenewableDispatch, "113_PV_1", (min = -30.0, max = 30.0))
+        (ThermalStandard, "115_STEAM_3", (min = -50.0, max = 100.0))
+        (ThermalStandard, "207_CT_1", (min = -70.0, max = 70.0))
+        (RenewableDispatch, "215_PV_1", (min = -40.0, max = 40.0))
+        (ThermalStandard, "307_CT_1", (min = -70.0, max = 70.0))
+        (ThermalStandard, "315_CT_8", (min = 0.0, max = 80.0))
+    ]
+        set_reactive_power_limits!(
+            get_component(component_type, sys, component_name),
+            new_limits,
+        )
+    end
 
-# TODO more hacks
-PowerSystems.get_r(::TwoTerminalHVDCLine) = 0.001
-PowerSystems.get_x(::TwoTerminalHVDCLine) = 0.0
-PowerSystems.get_b(::TwoTerminalHVDCLine) = (from = 0.0, to = 0.0)
+    # Patch https://github.com/NREL-Sienna/PowerFlows.jl/issues/47
+    sync_conds = filter(c -> occursin("SYNC_COND", get_name(c)),
+        collect(get_components(StaticInjection, sys)))
+    set_base_power!.(sync_conds, 100.0)
+    return sys
+end
 
 "Take RTS_GMLC_DA_sys and make some changes to it that are fully captured in the PowerFlowData(ACPowerFlow(), ...)"
 function modify_rts_system!(sys::System)