Skip to content
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

HybridACPowerFlow #97

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

HybridACPowerFlow #97

wants to merge 35 commits into from

Conversation

luke-kiernan
Copy link
Collaborator

With @rbolgaryn:

  1. Added an abstract supertype LinearSolverCache for all linear solvers, so there's a uniform interface for symbolic and numeric factoring. Right now, there's 2 subtypes: KLULinSolveCache (used in code) and LULinSolveCache (mainly for testing purposes).
  2. Added a new solver type, HyrbidACPowerFlow in nlsolve_powerflow.jl: a simple Newton linear search, storing the residual and Jacobian as functors, using KLU to invert the Jacobian. This is faster than NLSolveACPowerFlow by about 25%, though it's a bit less robust (errors at non-invertible J, convergence issues: may want to switch to a trust region method eventually).
  3. Tests for both of the above things: added similar code for Hyrbid wherever NLSolveACPowerFlow appears in the tests. These all passed, except for a pair (in test_psse_export.jl: I've commented them out) where Hybrid fails to converge. (NLSolve.jl's newton method also fails to converge for that same data, so it's limitation of the method, not a bug.) Also added test_klu_linear_solver.jl.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

JuliaFormatter

[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶

PF.symbolic_factor!(singCache, sing)


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶


[JuliaFormatter] reported by reviewdog 🐶

(NLSolveACPowerFlow, KLUACPowerFlow, HyrbidACPowerFlow)

JuliaFormatting suggestions

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
# update jacobian.
J(nlCache.x)

converged =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move convergence check to a function

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented here

Copy link
Member

@jd-lara jd-lara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if we might need to do a refinement step in the KLU solve to make NR more robust

@jd-lara
Copy link
Member

jd-lara commented Feb 19, 2025

Why did you call this Hybrid?

@luke-kiernan
Copy link
Collaborator Author

luke-kiernan commented Feb 19, 2025

I called it Hybrid because it combines the KLU factorization with the functors of NLSolve.jl. I originally had it as NLSolveACPowerFlow (and renamed the current NLSolveACPowerFlow to NLSolveACPowerFlowOld), but it seemed like I should give it some other name, to avoid confusion.

Copy link
Contributor

@GabrielKS GabrielKS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some preliminary code quality/style comments.

cache.K._symbolic = C_NULL
@assert cache.rf_symbolic[] == C_NULL
end
if cache.K._numeric != C_NULL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is accessing the _symbolic and _numeric fields of K part of KLU's public interface? If not (and my guess is not), find a way to avoid

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a subject of discussion with @rbolgaryn. We're doing things this way (directly calling C wrappers, using private fields) for performance reasons: KLU's public interface was not written in a non-allocating manner. That being said, I have not actually measured the performance difference.

Copy link
Contributor

@GabrielKS GabrielKS Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @jd-lara and @daniel-thom want to weigh in here but I'd be veeery wary of doing this. I guess you could check the Git history to find out how stable in practice some of these things are, but typically when a field starts with an underscore it's to explicitly signal that it should not be relied upon externally….

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, how hard would it be to write our own wrapper to replace KLU.jl?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it looks in the KLU.jl package:

function klu_factor(Ap, Ai, Ax, Symbolic, Common)
    ccall((:klu_factor, libklu), Ptr{klu_numeric}, (Ptr{Int32}, Ptr{Int32}, Ptr{Cdouble}, Ptr{klu_symbolic}, Ptr{klu_common}), Ap, Ai, Ax, Symbolic, Common)
end


function klu_solve(Symbolic, Numeric, ldim, nrhs, B, Common)
    ccall((:klu_solve, libklu), Cint, (Ptr{klu_symbolic}, Ptr{klu_numeric}, Int32, Int32, Ptr{Cdouble}, Ptr{klu_common}), Symbolic, Numeric, ldim, nrhs, B, Common)
end

function klu_refactor(Ap, Ai, Ax, Symbolic, Numeric, Common)
    ccall((:klu_refactor, libklu), Cint, (Ptr{Int32}, Ptr{Int32}, Ptr{Cdouble}, Ptr{klu_symbolic}, Ptr{klu_numeric}, Ptr{klu_common}), Ap, Ai, Ax, Symbolic, Numeric, Common)
end

@@ -366,28 +366,28 @@ function _update_F!(F::Vector{Float64}, Sbus_result::Vector{Complex{Float64}},
return
end

function _update_dSbus_dV!(rows::Vector{Int64}, cols::Vector{Int64},
function _update_dSbus_dV!(rows::Vector{Int32}, cols::Vector{Int32},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did we land on Int64 vs. Int32?

luke-kiernan and others added 11 commits February 20, 2025 13:32
…alculations, filter init voltage guess, implement loss factors for HybridPowerFlow, some additional minor changes. TODO: figure out how to handle the PSSE roundtrip testsn that are failing because of the voltage guess correction.
Newton AC PF enhancements: loss factor calculation, progress bar option, better filtering of initial guesses, and some code style tweaks for the new `HybridPowerFlow`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants