Conversation
abirchfield
left a comment
There was a problem hiding this comment.
Looks good. This will be needed for the larger case models.
494b6a6 to
f1cf35b
Compare
pelesh
left a comment
There was a problem hiding this comment.
I guess I don't understand the intent of this PR. In my view it would make more sense to have mva_base_system_ and mva_base_local_ parameters and then convert units at the component-system interface.
Magic numbers are an error prone way to go about this.
src/Model/PhasorDynamics/SynchronousMachine/GenClassical/GenClassicalImpl.hpp
Outdated
Show resolved
Hide resolved
src/Model/PhasorDynamics/SynchronousMachine/GenClassical/GenClassicalImpl.hpp
Outdated
Show resolved
Hide resolved
src/Model/PhasorDynamics/SynchronousMachine/GENROUwS/GenrouImpl.hpp
Outdated
Show resolved
Hide resolved
src/Model/PhasorDynamics/SynchronousMachine/GENROUwS/GenrouImpl.hpp
Outdated
Show resolved
Hide resolved
The intent of the PR is to implement a feature of generators (different MVA bases from the system base) that will be necessary for medium and large cases. Yes, I think that ideally we should have a system-wide parameter @lukelowry is that a change we can make easily? I guess I'm wondering whether the component has access to system parameters. |
|
I have added a private variable To clarify, this PR allows us to correctly implement larger cases (such as Hawaii), as the MVA bases for each generator differ in those cases. However, there is no general procedure to do base conversions, as each model/device may implement bases differently. An MVA base does not make sense for some models. This PR allows us to begin testing the Hawaii (& larger cases) against simulated transients from external tools (Power World) |
There is system setting for MVA base specified in the JSON input file format. This should be used to set the system MVA. Perhaps the best way to go about it is to have the system MVA as the member variable of the |
Prior to this PR, the system MVA base and any component MVA bases in the JSON input file are ignored. Effectively we assume the MVA base is 100 MVA everywhere. The purpose of this PR is to implement component-level MVA bases other than 100 MVA for GenClassical and GENROU, which we need for the next round of models. I do think a good future task would be to update both the parser and system composer to pass system MVA base into the component model, to allow hypothetical future cases with system MVA base other than 100 MVA. If it is simple we could go ahead and do it, but I think this could be an involved task as it would require changes to the core of the parser and system composer. For now, would it make sense to move forward with this PR with the current scope and create an issue for system MVA base to be addressed later? |
In that case, I suggest to move |
2c16a7e to
1078daf
Compare
Description
Add support for machine models to have a different MVA base than the system.
Proposed changes
mva_baseparameter to GenClassical and Genrouva_baseandfreq_basefrom the general Component class (every model handles this differently, and it could actually be misleading to offer this as an option for models like exciter and governor)Checklist
-Wall -Wpedantic -Wconversion -Wextra.Further comments
This is a minor fix and does not need to be included as a change feature.