-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PSSE Exporter Part 2: Integrate with PowerSimulations #48
Conversation
Tests depend on NREL-Sienna/InfrastructureSystems.jl#408 |
src/psse_export.jl
Outdated
@@ -59,6 +61,28 @@ function with_units(f::Function, sys::System, units::Union{PSY.UnitSystem, Strin | |||
end | |||
end | |||
|
|||
# TODO a bit hacky, there may be a better way (round trip JSON serialization is too slow). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniel-thom thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If people need this use case, I'm fine with it. Copying large numbers of time series arrays can be slow. Does it work, though? I'm wondering if the copied components have a reference to the original TimeSeriesManager. This code runs when you add a component to the system.
refs = SharedSystemReferences(;
time_series_manager = data.time_series_manager,
supplemental_attribute_manager = data.supplemental_attribute_manager,
)
set_shared_system_references!(component, refs)
The data will not be copied. But the component would have a reference to another system's time series manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some options:
- Reorganize the shared references object.
SystemData
could have a field calledshared_system_references
which containstime_series_manager
andsupplemental_attribute_manager
. When each component is added, it gets assigned a reference to that field. Then the time_series_manager is hot-swappable. - Keep the existing code but add a loop over all components and reassign
time_series_manager
andsupplemental_attribute_manager
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking, for my use case it should not be a problem that the resulting system's components have references to another system's time series manager, because systems I copy this way ought never be exposed to the user and my private use of them does not involve touching that stuff. But it's a sufficiently weird result that I'm willing to change it anyway. I don't really have the time/appetite to go mucking about with how we handle shared references right now (and it seems like an implementation detail we could easily change later anyway), so I'll go with option 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -1020,3 +1073,15 @@ function PSY.System(raw_path::AbstractString, md::Dict) | |||
# TODO remap everything else! Should be reading all the keys in `md` | |||
return sys | |||
end | |||
|
|||
# TODO handle kwargs | |||
make_power_flow_container(pfem::PSSEExportPowerFlow, sys::PSY.System; kwargs...) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you planning this into the public API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're still planning to not expose PowerFlowData
to the public then no, since the other methods of this return a PowerFlowData
. But I've been thinking that we might need like a "private/internal but stable" API as well as a public API — I need a stable function like this to be able to call from PowerSimulations.
Merging this to proceed with a review of all the recent changes ensemble. |
3a8aced
to
8b8af1f
Compare
This PR continues the effort from #42 and #29 (companion to NREL-Sienna/PowerSimulations.jl#1040). Here we seek to create a standardized interface to either run a PF power flow or export a PSS/E file for each iteration of a PSI simulation.