modelica-3rdparty / PowerSystems

Free (standard conform) library that is intended to model electrical power systems at different levels of detail both in transient and steady-state mode.
67 stars 36 forks source link

EPS: a lab for Power Systems Library #28

Closed ceraolo closed 7 years ago

ceraolo commented 7 years ago

I contribute to Power System Library with tickets. But I want to do more.

Therefore I've changed it whenever I deemed useful, and uploaded the modified copy into gitHub. It is available at: https://github.com/ceraolo/Electric-Power-Systems-EPS

Note that some explanation of how Power Systems Library works (that I miss in the embedded documentation); and on the modifications that I propose to issues I see, is reported in file added in the root of the library: "Basic explanation.docx". this file will be continuously updated.

In my idea EPS should work as a laboratory for experimenting improvements on the Power Systems Library. I hope that whatever change proposed in EPS is deemed useful and acceptable by the people that maintain PSL, will be incorporated there. Also whatever part of Basic explanation.docx is deemed of interest may be freely added to PSL.

Suggestions and comments are welcome directly on the EPS GitHub area.

tshort commented 7 years ago

It looks like you didn't do a true git fork. You lost all git history. In addition to being impolite to the original developers, it's tough to figure out what changes you made. A true fork would be much better. You'll also have the most success in getting changes in the upstream PSL if you take the initiative to create pull requests.

On another topic, I'd avoid docx for documentation. Markdown is common. I can't browse docx from the web interface of github.

rfranke commented 7 years ago

You might consider the overall structure of the lib given to start with. Pull requests are a perfect tool for e.g. #21 and #26. Improved documentation is another ever important area. Issues are more appropriate if you don't exactly know how to fix something, like #18.

Per unit is certainly needed for electrical power systems. This is why it does not help to remove p.u. from the library and I don't think that the solution found is that bad. If you don't agree, then please contribute to the long-standing Modelica ticket.

The placement of the System model in the top scope of the lib is common Modelica practice. See e.g. Modelica.Mechanics.MultiBody.World and Modelica.Fluid.System. No good idea to place it under Utilities.

ceraolo commented 7 years ago

To tshort I'm sorry for having done something looking impolite. The reason for doing this way because I wanted to be free to make several changes, and show people the outcome, to generate ideas for something to be later added to the original library. Naturally I could have suggested specific changes with pull requests to the original library, but this would have involved a much more rigid path.

Sure, I can do the other way, deleting this "EPS" and resorting to pull requests when I have some specific to suggest. I will think about doing this. For the time being I just change dhe docx into pdf.

tshort commented 7 years ago

To be clear, I wasn't trying to criticize your fork. Forking is a great way to experiment with the library. I was criticizing the way you did your fork. By just copying files and re-uploading them, your lost all of the git history. A better approach to take is to clone this repository, make any changes you want, then push that to a new repository with git (your fork). That way, the complete git history is kept, and others can see what you changed along the way. You are not stuck just doing pull requests. You have the same freedom you have now, but the history is maintained. This also makes it easier for upstream (this repo) to cherry-pick parts of your changes.

ceraolo commented 7 years ago

Thanks for you explanation and suggestions, that I will surely follow (in a few weeks: now I have no more time for Power Systems Library). However I've just uploaded a file "Basic explanation work-in-progress.pdf" in which I do two things: 1) try to contribute to enhance the documentation of the Library. I had serious difficulties in envisaging how it works; and therefore I wrote here a few notes that should help others to avoid to make the same path as me 2) describe some of the changes (in my view enhancements) to the library. The file is "work-in-progress", since it is by far incomplete.

I agree with you that having lost all history was a mistake and I will follow your suggestions; however the pdf maybe can be useful to someone already.

rfranke commented 7 years ago

It would be helpful if you contributed 1. to the PowerSystems user's guide (UsersGuide.mo), ideally in the form of a pull request. Also some infos given in 2. "work-in-progess" pdf file might be helpful there.

Your changes/enhancements certainly raise issues. They cannot be taken over though.

It is impossible to replace "Ohm/(V.V/VA)" with "ohm or p.u." because the latter is no valid Modelica unit. The ambition is that the library passes a pedantic Modelica check. Commit 545c30ef391ccd8ab415fdd0f0aff87051dfe80f introduces "Ohm/Ohm" as alternative.

The improvement proposal #26 did not work out for the same reason -- cannot access a conditional component outside connections. Commit 12da468d4d8837000c26343ca7b505a2b9656980 provides an alternative by making the phasors unconditional.

The Generic modules are both: generic and simple. They are currently declared as:

package Generic "Simple components for basic investigations"

Your proposal

package SimplifiedLib "Simple components for basic investigations"

looses the info that they are generic, i.e. work with any phase system. I hope that the new structure of the Examples, splitting Examples.Spot into Examples.AC1ph_DC and Excamples.AC3ph, beside Examples.Generic solves your issue without renaming Generic to SimplifiedLib.

ceraolo commented 7 years ago

I've done something to accommodate objections by tshort (that has received two likes): 1) I deleted the library that I called EPS and created a new called "Modified Power Systems" to indicate from the very name the link with the original Power System Library 2) I said in the very first row of the description that it is modification of Power System Library since a precise date 3) I made my modifications keeping the history and indicated in the presentation which modifications I've made that are experiments for proposal for the original Power System Library 4) I'm continuing with the effort of contributing to the original Power System Library with tickets (I will add another two today) and private messages to Ruediger. So the "Modified Power Systems" library is a lab to see at work modifications which are possible candidates for the original Power System Library. And it is a special version of the Library, which I like most and recommend to my students.

In this way, for me, the history is saved: history up to Jan, 20th because I indicated the precise date for the derivation from PSL, and after that date, since and every single modification made from that point on is registered. I did not completely follow the suggestion from tshort, because this way I have a more clear indication of what I'm doing.

The new link is as follows: https://github.com/ceraolo/Modified-Power-Systems

Regards.

dietmarw commented 7 years ago

@ceraolo Unfortunately you are still missing the point of what is meant by history. It is the git commit history that is lost and also your library will not be displayed as a fork. You really should go back and fork the library and then add your changing commits. Forking is very simply via a the fork button on the top.

dietmarw commented 7 years ago

Just as an example, I've just forked the library (took one click and 5 sec for GitHub to process). I now have: image displayed on my fork whilst yours is displayed as a stand alone project. image When you do it that way, any of your commits will also appear on: https://github.com/modelica/PowerSystems/network so people can see where what development is going on.

dietmarw commented 7 years ago

Btw, I've just took your changes and rebased them onto the original PowerSystems using my fork. You can now look at the network link of the branch ModifiedPowerSystems how this looks like. If you want I can transfer my fork to your account, so you don't have to do it again.

ceraolo commented 7 years ago

Well, since you repeat insist, and you helped with some work, I will operate according to your suggestions.

Just a technical question. In the branch you created for me, I see exactly what I had on my previous area named "Modified-Power-Systems". This is what I would like to have after transferring you fork to my account. What steps should I follow to transfer transfer your fork to my account? If I click "fork" from your fork I see in the first page the original description of the library, not my "readme.md".

dietmarw commented 7 years ago

@ceraolo The correct way to proceed is:

  1. remove (i.e., delete) the PowerSystems fork you created earlier https://github.com/ceraolo/PowerSystems/settings (at the bottom)
  2. once you done this give me a shout out and I will transfer my copy to you
  3. done :-)
ceraolo commented 7 years ago

I did step 1. Will you proceed with step 2?

dietmarw commented 7 years ago

Sorry but https://github.com/ceraolo/PowerSystems still exists and I can not transfer my fork until you have removed (i.e., deleted) it via the settings

ceraolo commented 7 years ago

OOPS! I've just deleted this as well. Thank you for your assistance.

dietmarw commented 7 years ago

OK you should have gotten a transfer request. Your changes in that repository are on the "ModifiedPowerSystems" branch. And I would keep them there also in the future. That way you can keep the master of your fork in sync with "upstream" while at the same time working on your version in its dedicated branch.

ceraolo commented 7 years ago

It works! Thank you.

ceraolo commented 7 years ago

I've started with my project of making my modified library grow according to my ideas. Now I think this ticket can be closed.