CURENT / andes

Python toolbox / library for power system transient dynamics simulation with symbolic modeling and numerical analysis 🔥
https://ltb.curent.org
Other
223 stars 111 forks source link

Prepare for new minor version release. #180

Closed cuihantao closed 2 years ago

codecov[bot] commented 2 years ago

Codecov Report

Merging #180 (fd03185) into master (1aee09a) will increase coverage by 0.19%. The diff coverage is 87.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   78.57%   78.76%   +0.19%     
==========================================
  Files         108      110       +2     
  Lines       11872    12036     +164     
==========================================
+ Hits         9328     9480     +152     
- Misses       2544     2556      +12     
Impacted Files Coverage Δ
andes/models/__init__.py 100.00% <ø> (ø)
andes/core/block.py 82.94% <62.90%> (-0.85%) :arrow_down:
andes/models/exciter/__init__.py 100.00% <100.00%> (ø)
andes/models/exciter/ac8b.py 100.00% <100.00%> (ø)
andes/models/exciter/ieeet3.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1aee09a...fd03185. Read the comment docs.

cuihantao commented 2 years ago

@jinningwang Hi JInning,

I'm looking to release a new minor version, but the PIDController model has a lot of duplication with the PIController block. Can you refactor it and let it inherit from PIController?

You can easily overwrite equations that are defined in PIController in the define() function with direct access to the e_str attribute.

cuihantao commented 2 years ago

@jinningwang Also, there seems to be an error in the definition of PIDAWHardLimit. It should inherit from PIDController instead of PIController. Python allowed you to call the constructor of PIDController, however.

Why does the model need a hard limiter at the output? If it is the case, there needs to be another limiter-like symbol _/- following the output.

jinningwang commented 2 years ago

@jinningwang Also, there seems to be an error in the definition of PIDAWHardLimit. It should inherit from PIDController instead of PIController. Python allowed you to call the constructor of PIDController, however.

Why does the model need a hard limiter at the output? If it is the case, there needs to be another limiter-like symbol _/- following the output.

Hi Hantao,

Will take care of the code tomorrow.

As for the hard limiter, basically I just follow the existing PIAWHardLimit. In such way, the total output of PID will be limit by HardLimiter, while the integrator part of PID output will be limited by AntiWindup. Oh, I just realized that, maybe we also need a AntiWindup for the Washout part?

Regards, Jinning

cuihantao commented 2 years ago

Will take care of the code tomorrow.

Thanks!

As for the hard limiter, basically I just follow the existing PIAWHardLimit. In such way, the total output of PID will be limit by HardLimiter, while the integrator part of PID output will be limited by AntiWindup.

In AC8B, the block is not a PIDAWHardLimit, but should be a PIDAW. The PIAWHardlimit is used in WTPTA1, which on purpose included a hardlimiter at the output. See https://www.powerworld.com/WebHelp/Content/TransientModels_HTML/Stabilizer%20WTGPT_A.htm.

For AC8B, maybe you can implement a PIDTrackAW that derives from PITrackAW. The PITrackAW uses an internal state for tracking the anti-windup process and implements a hard limiter internally already. You can add a washout to PITrackAW, let the washout's output sum up to ys before the limit, without having to modify the HardLimiter.

For more details about the Track AW, see the Appendix in Milano's book.

jinningwang commented 2 years ago

@cuihantao Hi Hantao,

I have modified the code regarding our yesterday discussion. The change is in the new PR. The PR failed to pass the checks, but I don't know why. ANDES on my laptop runs with no error and can pass selftest. Would you take a look at the PR?

Regards, Jinning

sonarcloud[bot] commented 2 years ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 20 Code Smells

No Coverage information No Coverage information
15.2% 15.2% Duplication

jinningwang commented 2 years ago

Hi Hantao,

I noticed the duplication rate is too high, I'll try to refactor some of the exciters.

Regards, Jinning

cuihantao commented 2 years ago

Yeah, that’s what I asked about. See my previous note on overwriting equation after inheriting.

Regards, Hantao Cui On Sep 26, 2021, 5:53 PM -0500, Jinning Wang @.***>, wrote:

Hi Hantao, I noticed the duplication rate is too high, I'll try to refactor some of the exciters. Regards, Jinning — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.