Update current pti parser to handle V35 PSSE files by mcllerena · Pull Request #1419 · Sienna-Platform/PowerSystems.jl

mcllerena · 2025-06-16T05:59:11Z

Current parser is extracting correctly substation location data for parse_pti and parse_psse
Handle of multi-line Re/Im of IC data for v35.
Add to ext new parameters of version v35.
Update Load and Switched Shunt structs to handle old and current file versions.

jd-lara

The biggest challenge here is the addition of the interruptible field because we have both interruptible and non-interruptible loads that can be modeled differently and the addition of the flag is not consistent with that pattern. When loads are interruptible we should create the correct type of interruptible because in PCM those require the correct implementation.

…type when creating the system

mcllerena · 2025-06-16T19:57:34Z

Added these fixes on the last commits, did the tests changing the interruptible flag on the raw file and the load types were created correctly after creating the system.

jd-lara

Aren't we converting the substation geospatial data into the GeographicInfo attribute?

mcllerena · 2025-06-17T18:20:08Z

Aren't we converting the substation geospatial data into the GeographicInfo attribute?

I initial just parse it and store it on parse_pti and parse_psse. I can include them as a GI attribute, I'll do a new commit addressing this.

mcllerena · 2025-06-18T05:24:02Z

Just finished adding the GI to the buses 69a0aec

jd-lara

Add a new structure that combines interruptible power + standard load to support PSSe interruptible flag.

jd-lara · 2025-06-30T01:11:17Z

Add a new structure that combines interruptible power + standard load to support PSSe interruptible flag.

@mcllerena I made a new PR with the additional object we needed.

interruptible standard load

Copilot

Pull Request Overview

This PR updates the PTI parser to support V35 PSSE files and improves handling of multi‐line Re/Im data and additional V35 parameters. Key changes include:

Introducing a new InterruptibleStandardLoad in the tests.
Adding geographic information to buses and adjusting load and transformer parsers for V35.
Updating data type definitions and PSSE parsing logic to incorporate new V35 fields.

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/test_constructors.jl	Adds tests for the new InterruptibleStandardLoad constructor
src/parsers/power_models_data.jl	Enhancements for substation geographic info and updated load builders
src/parsers/pm_io/pti.jl	Adjustments in PTI parsing to distinguish V35 files and multi‐line data
src/parsers/pm_io/psse.jl	Updates to PSSE parsing logic to handle new V35 field names and formats
src/descriptors/power_system_structs.json	New fields and updated defaults for V35 load models
src/PowerSystems.jl	Exports the new InterruptibleStandardLoad

Copilot · 2025-06-30T20:38:48Z

+            end
+        end
+
+        @info "Added coordinates to $(buses_with_coords[]) buses, $(buses_without_coords[]) buses without coordinates"


The use of empty bracket indexing (buses_with_coords[] and buses_without_coords[]) for what appear to be integer counters is unusual. If this is not intended for array access, consider removing the brackets to simply interpolate the integer values.

Suggested change

@info "Added coordinates to $(buses_with_coords[]) buses, $(buses_without_coords[]) buses without coordinates"

@info "Added coordinates to $(buses_with_coords) buses, $(buses_without_coords) buses without coordinates"

I agree with Copilot.

Copilot · 2025-06-30T20:38:49Z

        if haskey(pti_data, section)
-            push!(pti_data[section], section_data)
+            if section == "IMPEDANCE CORRECTION" &&
+               pti_data["CASE IDENTIFICATION"][1]["REV"] == 35


[nitpick] The conditional branch for the 'IMPEDANCE CORRECTION' section when REV equals 35 causes the parser to skip adding the parsed section data. Please review this logic to confirm if this behavior is intentional; if so, adding a clarifying comment would improve maintainability.

Suggested change

pti_data["CASE IDENTIFICATION"][1]["REV"] == 35

pti_data["CASE IDENTIFICATION"][1]["REV"] == 35

# Skipping the addition of parsed data for "IMPEDANCE CORRECTION" when REV equals 35.

# This behavior is intentional and based on the specific requirements of PTI v35 files.

daniel-thom · 2025-06-30T23:33:36Z

+            lat, lon = substation["latitude"], substation["longitude"]
+            for node in substation["nodes"]
+                if haskey(node, "I")
+                    bus_coords_lookup[node["I"]] = (lat, lon)


Could you create the GeographicInfo object here and share references across components? It could matter a lot if there are many instances of components at the same location.

Agree with this

Fixed on last commits

daniel-thom · 2025-06-30T23:34:26Z

+            end
+        end
+
+        @info "Added coordinates to $(buses_with_coords[]) buses, $(buses_without_coords[]) buses without coordinates"


I agree with Copilot.

daniel-thom · 2025-06-30T23:36:47Z

+            load = make_interruptible_standardload(d, bus, sys_mbase; kwargs...)
+            has_component(InterruptibleStandardLoad, sys, get_name(load)) && throw(
+                DataFormatError(
+                    "Found duplicate interruptible load names of $(get_name(load)), consider formatting names with `load_name_formatter` kwarg",


Prefer summary(load) over get_name(load) because it includes the struct name.

daniel-thom · 2025-06-30T23:54:06Z

+
+        return updated_line_index
+    catch message
+        throw(@error("Parsing failed at line $line_index: $(sprint(showerror, message))"))


This doesn't look right. Do you mean to call error()?

Yes, my bad. I just fix it on the last commits.

Looks like the pattern preceded you. It's odd, but still kinda works.

julia> throw(@error("asdfs")) ┌ Error: asdfs └ @ Main REPL[15]:1 ERROR: nothing Stacktrace: [1] top-level scope @ logging/logging.jl:418

The ERROR: nothing is confusing though.

Yeah, better to use the error() or @error

@mcllerena @error is for logging an error not throwing

daniel-thom · 2025-07-01T00:18:15Z

+    current_dtypes = is_v35 ? _pti_dtypes_v35 : _pti_dtypes
+
+    line_index = 1
+    while line_index <= length(data_lines)


There is a lot of logic here and I can't say that I've reviewed all of it. I will say that I'm unsure about your change from a for loop (built-in index) to a while loop (managed index). It will be very easy for someone to miss incrementing line_index in some conditional. What is your reasoning here?

Yes, I was aware of this risk in using a while loop instead of a for loop. Is just that because I had to keep track of the line index for each components, where there were line/multi-line structures like in the Impedance Correction Data for v35 files, also for the complex parsing of the Substation Data Blocks structure in the file to extract the latitude/longitude fields; I couldn't find a solution to handle this with a for loop when I wrote the code.

Ok thanks, I see that now. Can you point me at an example? I'd like to take a look at one of these files before approving.

Sure, I'll share it internally since the only one I have with all these characteristics is really heavy.

@mcllerena we can merge once Dan Approves but we still haven't implemented extensive testing. We should add an issue for increasing testing before release.

I’ll be building small tests with ICD and substation data specially, since these were the most annoying ones

mcllerena added 11 commits June 14, 2025 14:27

remove duplicated print statement at the beginning of parsing

c2c839d

parse pti file for psse v35

c82edfe

add substation data coordinates to the pti_data section dict

df7c1d8

add parsing of IC data for v35 files

8647b56

run formatter and update pti fields

532a77a

update psse parsing for v35

4e96d80

update loads and switched shunts structs

ff9b876

update psse parser and power models data to create v35 system

8f0476a

run formatter

6999bd4

update handling of IC data for new psse versions

a45b7c8

run formatter

c3530aa

mcllerena requested review from GabrielKS, SebastianManriqueM, jd-lara, m-bossart and rodrigomha June 16, 2025 05:59

jd-lara reviewed Jun 16, 2025

View reviewed changes

Comment thread src/descriptors/power_system_structs.json

jd-lara reviewed Jun 16, 2025

View reviewed changes

Comment thread src/descriptors/power_system_structs.json

jd-lara reviewed Jun 16, 2025

View reviewed changes

Comment thread src/descriptors/power_system_structs.json Outdated

jd-lara requested changes Jun 16, 2025

View reviewed changes

jd-lara self-assigned this Jun 16, 2025

jd-lara added the PowerSystems 5.0 label Jun 16, 2025

mcllerena linked an issue Jun 16, 2025 that may be closed by this pull request

Update/Implement parser to handle V35 PSSE files #1405

Closed

mcllerena added 2 commits June 16, 2025 13:48

remove interruptible flags from loads to replace it with the correct …

4fcd033

…type when creating the system

separate interruptible and non-interruptible loads by the correct type

d1ab1ac

mcllerena requested a review from jd-lara June 16, 2025 19:57

jd-lara requested changes Jun 17, 2025

View reviewed changes

update parse_pti to store nodes of each substation entry

f7540ba

mcllerena requested a review from jd-lara June 18, 2025 05:24

jd-lara requested changes Jun 23, 2025

View reviewed changes

jd-lara added 2 commits June 29, 2025 16:17

add interruptible standard load

73968f6

use interruptible standard load in psse parser

1d14d22

jd-lara added 6 commits June 30, 2025 12:38

fix constructor

0e65a0a

fix interruptible power load

ecb4f0c

one more fix

59bfe7f

fix constructor call

e0e3413

fix the has component

457dfb9

Merge pull request #1444 from NREL-Sienna/jd/interruptible_standard_load

862e52d

interruptible standard load

kdayday mentioned this pull request Jun 30, 2025

Jd/psy45 docsv3 #1446

Merged

jd-lara requested review from Copilot and daniel-thom and removed request for SebastianManriqueM, m-bossart and rodrigomha June 30, 2025 20:38

Copilot AI reviewed Jun 30, 2025

View reviewed changes

mcllerena and others added 2 commits June 30, 2025 17:13

Merge branch 'psy5' into ml/parse_v35_psse_files

4cef7b9

run formatter

e6cd00d

daniel-thom reviewed Jun 30, 2025

View reviewed changes

mcllerena added 2 commits June 30, 2025 18:00

fix error message in pti file

1fe13c4

update geo info function for creating the supplemental attributes

93d515d

mcllerena requested a review from daniel-thom July 1, 2025 00:02

daniel-thom reviewed Jul 1, 2025

View reviewed changes

daniel-thom approved these changes Jul 1, 2025

View reviewed changes

jd-lara merged commit e59ff31 into psy5 Jul 1, 2025
1 of 9 checks passed

jd-lara deleted the ml/parse_v35_psse_files branch July 17, 2025 16:28

	@info "Added coordinates to $(buses_with_coords[]) buses, $(buses_without_coords[]) buses without coordinates"
	@info "Added coordinates to $(buses_with_coords) buses, $(buses_without_coords) buses without coordinates"

Uh oh!

Conversation

mcllerena commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jd-lara left a comment

Choose a reason for hiding this comment

Uh oh!

mcllerena commented Jun 16, 2025

Uh oh!

jd-lara left a comment

Choose a reason for hiding this comment

Uh oh!

mcllerena commented Jun 17, 2025

Uh oh!

mcllerena commented Jun 18, 2025

Uh oh!

jd-lara left a comment

Choose a reason for hiding this comment

Uh oh!

jd-lara commented Jun 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants