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

PSAT AVR models mixed up. #164

Closed
dietmarw opened this issue Apr 3, 2018 · 8 comments · Fixed by #233
Closed

PSAT AVR models mixed up. #164

dietmarw opened this issue Apr 3, 2018 · 8 comments · Fixed by #233
Assignees
Milestone

Comments

@dietmarw
Copy link
Member

dietmarw commented Apr 3, 2018

When working with the AVR models from the PSAT package we noticed the following:

OpenIPSL.Electrical.Controls.PSAT.AVR.AVRTypeI
and
OpenIPSL.Electrical.Controls.PSAT.AVR.AVRTypeII

are actually swapped. So AVRTypeIshould be AVRTypeII and vice versa. Now the real tricky bit is here that also PSAT got this wrong. If you look at the official IEEE definitions and literature (e.g., M. Federico, Power-System-Modelling-and-Scripting. New York: Springer Science & Business Media, 2010.) you'll see that the PSAT manual got it wrong and hence OpenIPSL "copied" this.

So rather than having the identical (wrong) naming from PSAT I'd suggest to swap the models around and document this in-consistency with regards to PSAT. Of course this will have an impact on user models. Question is if it will not even fix user-models since they were not aware that they did not use the proper AVR type.

@MaximeBaudette
Copy link
Member

@dietmarw
My first reaction to your post is that, given that our user community is not that big, it's not such a big deal if we swap the names to fix it w.r.t the standard.

@dietmarw
Copy link
Member Author

dietmarw commented Apr 4, 2018

We could actually solve this with a conversion script so that user-models get updated. But this change being non-backwards compatible would mean that the semantic version of the next release should be 2.0.0. Maybe there are some other more major changes that are planned and should be bundled with such a release. Not sure what your release plan looks like for OpenIPSL.

@MaximeBaudette
Copy link
Member

I don't think we have a very defined plan at the moment, and for my part, I'm pretty busy these days, so I don't spend too much time on OpenIPSL. But I think @lvanfretti is getting new students who are gonna work on the library very soon! Hopefully the second half of 2018 will see a more busy repository :-)

@lvanfretti
Copy link
Member

@MaximeBaudette this is something that we have to put in the to-do list for @marcelofcastro who will take over your admin and main dev role.
@dietmarw I like the idea of the script, Marcelo starts in the end of August, let's follow up then and talk about future releases. For the time being, I guess this could be "patched" manually.

@MaximeBaudette MaximeBaudette added this to the 2.0.0 milestone Jul 2, 2018
@marcelofcastro
Copy link
Member

Guys, was this issue solved? Did we changed the names?

@dietmarw
Copy link
Member Author

I need to check. Probably will manage that next week.

@dietmarw
Copy link
Member Author

I had a look this morning. So nothing was changed for now. The thing really is to decide how the PSAT models should be used. Given the name the idea is probably to be able to recreate PSAT examples using OpenIPSL. In that case it would be unfortunate that the AVR types are different wrt PSAT even though more correct wrt the original IEEE source.
So my suggestion is to maybe simply document inside these two models and pointing out the issue. I'd even go so far to add a small message in red on the diagram level.

@lvanfretti
Copy link
Member

lvanfretti commented May 22, 2020 via email

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 a pull request may close this issue.

4 participants