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

Fix PSSE parser of WTDTA1 #279

Merged
merged 11 commits into from
Feb 25, 2022
Merged

Fix PSSE parser of WTDTA1 #279

merged 11 commits into from
Feb 25, 2022

Conversation

jinningwang
Copy link
Member

@jinningwang jinningwang commented Feb 24, 2022

  • Fix equations of WTDTA1
  • Fix WTDTA1 in PSSE parser
  • Add non_negative=True to the param Mass in GENBASE, WTDTA1, WTDS

@jinningwang
Copy link
Member Author

Hantao,

I compared the PowerWorld WTDTA1 and our implementation of WTDTA1, and I think that the equation of WTDTA1 in the PSSE parser should be modified.
image
Do I miss anything else?

Regards,
Jinning

@codecov
Copy link

codecov bot commented Feb 24, 2022

Codecov Report

Merging #279 (ac9de6a) into develop (d4ecf4c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #279   +/-   ##
========================================
  Coverage    78.82%   78.82%           
========================================
  Files          152      152           
  Lines        12917    12918    +1     
========================================
+ Hits         10182    10183    +1     
  Misses        2735     2735           
Impacted Files Coverage Δ
andes/models/renewable/wtds.py 100.00% <ø> (ø)
andes/models/synchronous/genbase.py 89.33% <ø> (ø)
andes/models/renewable/wtdta1.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 d4ecf4c...ac9de6a. Read the comment docs.

@cuihantao
Copy link
Collaborator

Thanks for the PR!

Can you modify the PSSE parser to allow the use of a variable, sys_f, in any equation? This seems to be a much needed feature.

@cuihantao
Copy link
Collaborator

I made a mistake in the original equation by forgetting about the square. Can you analyze the control diagram and figure out if Freq1 need to be converted to per unit? Recall that in WTDTA1 in ANDES, all other quantities such as H, D, omega are in per unit in system base. The unit of Freq1 should be consistent for the swing equations.

@jinningwang
Copy link
Member Author

Thanks for the PR!

Can you modify the PSSE parser to allow the use of a variable, sys_f, in any equation? This seems to be a much needed feature.

Sure thing, let me try some solutions.

@jinningwang
Copy link
Member Author

I made a mistake in the original equation by forgetting about the square. Can you analyze the control diagram and figure out if Freq1 need to be converted to per unit? Recall that in WTDTA1 in ANDES, all other quantities such as H, D, omega are in per unit in system base. The unit of Freq1 should be consistent for the swing equations.

I'll check this.

@jinningwang
Copy link
Member Author

Hantao, although PSSE doc said it is nominal frequency, I think per unit is more reasonable. In the test dyr file, the variable is given in per unit.
image

Regards,
Jinning

@cuihantao
Copy link
Collaborator

cuihantao commented Feb 24, 2022

Hantao, although PSSE doc said it is nominal frequency, I think per unit is more reasonable. In the test dyr file, the variable is given in per unit.

Something might be wrong here. Either the doc or PSS/E's not following the doc.

Anyway, what I asked you to do is to perform a rigorous analysis of the swing equation to reason if Freq1 needs to be in per unit or Hz. The manual is a reference, but an analysis is needed.

@jinningwang
Copy link
Member Author

Hantao, although PSSE doc said it is nominal frequency, I think per unit is more reasonable. In the test dyr file, the variable is given in per unit.

Something might be wrong here. Either the doc or PSS/E's not following the doc.

Anyway, what I asked you to do is to perform a rigorous analysis of the swing equation to reason if Freq1 needs to be in per unit or Hz. The manual is a reference, but an analysis is needed.

Sure, let me do this after fixing the initialization error of WTDTA1.

@sonarcloud
Copy link

sonarcloud bot commented Feb 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@jinningwang
Copy link
Member Author

jinningwang commented Feb 25, 2022

Wait, I noticed a bug, give me a moment.

Hantao,

I think this PR is ready to merge, all the issues related to WTDTA1 are solved:

  • The Freq1 should be in per unit after I analyze the equations, I add notices in the docstring of WTDTA1;
  • The initialization error is fixed when Kshaft=0.

The enhancement of adding sys_f in the PSSE parser can be done in another PR.

Regards,
Jinning

Copy link
Collaborator

@cuihantao cuihantao left a comment

Choose a reason for hiding this comment

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

Looks good.

andes/io/psse-dyr.yaml Outdated Show resolved Hide resolved
@cuihantao cuihantao merged commit 7803452 into CURENT:develop Feb 25, 2022
@jinningwang jinningwang deleted the voltosc branch March 26, 2022 18:55
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.

2 participants