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

Revision of power flow solver #284

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

martinmoraga
Copy link
Contributor

@martinmoraga martinmoraga commented Apr 15, 2024

Continuation of PR #113 (it was closed after the rebase of the master branch)

This PR fix some errors of the powerflow solver

@martinmoraga martinmoraga added the enhancement New feature or request label Apr 15, 2024
@martinmoraga martinmoraga self-assigned this Apr 15, 2024
@martinmoraga martinmoraga requested a review from gnakti April 24, 2024 11:01
python/src/dpsim/matpower.py Outdated Show resolved Hide resolved
python/src/dpsim/matpower.py Outdated Show resolved Hide resolved
@martinmoraga martinmoraga force-pushed the revise-pf-solver branch 5 times, most recently from c0d0766 to 1fcfa43 Compare May 21, 2024 11:00
Copy link

@martinmoraga martinmoraga requested a review from m-mirz May 21, 2024 19:51
dpsim/src/PFSolver.cpp Outdated Show resolved Hide resolved
Copy link

dpsim/src/PFSolver.cpp Outdated Show resolved Hide resolved
} else if (std::shared_ptr<CPS::SP::Ph1::Load> load =
std::dynamic_pointer_cast<CPS::SP::Ph1::Load>(comp)) {
baseVoltage_ = load->attributeTyped<CPS::Real>("V_nom")->get();
mSLog->info("Choose base voltage of {}V to convert pu-solution of {}.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe is better with SPDLOG_LOGGER_INFO(

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the macro variant can be optimized better if the log level is less than info.

std::dynamic_pointer_cast<CPS::SP::Ph1::NetworkInjection>(
comp)) {
baseVoltage_ = extnet->attributeTyped<CPS::Real>("base_Voltage")->get();
mSLog->info("Choose base voltage of {}V to convert pu-solution of {}.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think here is also SPDLOG_LOGGER_INFO(

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now there are 23 changed files. But it looks like in most of the files there are just a couple of lines with changed formatting. And a new formatting is different from the .clang-format rules, applied to the rest of the codebase.
It seem that the files with changes, that have influence, are:

  • SP_Ph1_NetworkInjection.h,
  • SP_Ph1_NetworkInjection.cpp,
  • PFSolver.cpp
  • PFSolverPowerPolar.cpp

I would then suggest to limit this PR to these 4 files (hope, I did not miss anything)

@@ -39,7 +39,7 @@ class NetworkInjection : public CompositePowerComp<Complex>,

// #### Powerflow section ####
/// Base voltage [V]
Real mBaseVoltage;
const Attribute<Real>::Ptr mBaseVoltage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see this is done the same way for the other components, but does mBaseVoltage really need to be of Attribute<Real>::Ptr type? ls there a specific functionality of Attributes that you'd like to use for mBaseVoltage, like logging, interfacing or task scheduling?
If you just want to get the value of mBaseVoltage in PFSolver.cpp a more readable solution could be to have a getter function:

public:
    // Getter for mBaseVoltage
    Real getBaseVoltage() const {
        return mBaseVoltage;
    }

@@ -321,6 +321,19 @@ void PFSolver::determineNodeBaseVoltages() {
"Choose base voltage {}V of {} to convert pu-solution of {}.",
baseVoltage_, gen->name(), node->name());
break;
} else if (std::shared_ptr<CPS::SP::Ph1::Load> load =
std::dynamic_pointer_cast<CPS::SP::Ph1::Load>(comp)) {
baseVoltage_ = load->attributeTyped<CPS::Real>("V_nom")->get();
Copy link
Collaborator

Choose a reason for hiding this comment

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

If mNomVoltage is intended to be of type Attribute::Ptr and is publicly accessible, consider directly accessing its value through the member variable instead of using a hard-coded string. For example:
baseVoltage_ = load->mNomVoltage->get()
This will ensure compile time-error checking. Now, if there is a typo in the string "V_nom" the error would not be caught at compile time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants