perf: Changes to ULTC and correct syntax fixes for OCT by hubertus65 · Pull Request #265 · OpenIPSL/OpenIPSL

hubertus65 · 2020-12-29T23:06:23Z

Added Boolean variables y2l0 and y3l0 and wrapped pre() around them in when-clasue condition. Result differs slightly for x2, m, y2, y3 after time = 300s. and looks more "logical" to me than the previous result in Dymola (Note: orginal version is rejected in OCT). Will attach plots in PR.

hubertus65 · 2020-12-29T23:10:32Z

Check of ULTC in Dymola before and after changes by HT, comparison:

Conclusion: I don’t know how this should look, but it looks “more correct” to me after my changes, i.e. wrapping a pre around y2 < 0 and y3 < 0. Please check! With pre is after my changes.

dietmarw · 2021-01-04T12:05:13Z

OpenIPSL/Examples/Solar/PSAT/SolarPQtest.mo

    t1=3,
    t2=3.1) annotation (Placement(transformation(
-        origin={20.0,-57.3},
+        origin={25,-57.3},


This origin annotation should be removed completely (since no rotation, there is no point in keeping the origin)

dietmarw · 2021-01-04T12:08:59Z

OpenIPSL/Examples/Solar/PSAT/testIrradiation.mo

-        origin={-87.9874,22.9501},
-        extent={{-3.3987,-3.3987},{3.3987,3.3987}})));
+        origin={0.0,0.0},
+        extent={{-100.04368118521606,39.95631881478396},{-79.95631881478394,60.04368118521604}},rotation = 0.0)));


These new annotations make it unfortunately worse and should not be added. In general it should be decided to maybe remove these Solar examples all together since it is not really clear what the purpose is of those. So rather then kind of fixing these it should be looked at removing them.

I actually did this change with Dymola, and did not start from scratch with graphics (Which I maybe should have). I think the odd "origin" annotations are from OpenModelica. That said: I would be in favor of removing the examples, they don't look well thought out, several look rather like unit tests than examples.

@lvanfretti: what is your take on this?

@hubertus65 yes, they are unit test examples for the solar models.

@dietmarw we don't have other PV models in the library, and we need to have unit tests at least for the existing ones. They are pretty good w.r.t. to models used in standard tools. They were developed at a time when the tools did not incorporate their own (in this case this was implemented both in PowerFactory, the reference is here).

Instead of removing them, we should actually move them under ./Examples/UnitTests as planned for v2 release.

Since the problem is only in the annotations, it should be easy enough for one of us to do it, we just have to assign someone. @GiuseppeLaera can help.

@GiuseppeLaera, I think the easiest way to fix this is to rebuild all PV examples in your favorite tool, but probably not in OpenModelica.

Actually, you do not need to rebuild but simply move the blocks back and forth and redo the connections that you should have clean annotations again. That's at least my experience when using Dymola 2020 onwards.

Just to add to this discussion, I have developed the PVD1 model according to PowerFactory.

Now I am working now on their own implementation of a PV model, which is I guess the one @dietmarw was referring to . I will also be adding some examples with those modes. A PR should follow soon.

@tinrabuzin I don't know exactly what Dietmar was referring too, but the only solar models we have is the ones from PSAT and the one from Farhan that I mentioned above. So, it will be great if you can commit the "official" PowerFactory one and even better if you can provide the reference trajectories from PF. Please get in touch with @marcelofcastro for examples on how we are doing the regression testing, which you can also see here:
here

@lvanfretti Yes, sorry it was you mentioning the model not Dietmar.

OpenIPSL/Icons/WorksInOCT.mo

OpenIPSL/Electrical/Branches/Generic/ULTC.mo

OpenIPSL/Electrical/Branches/PSAT/TwoWindingTransformer.mo

OpenIPSL/Electrical/Machines/PSAT/Order4.mo

dietmarw · 2021-01-28T15:32:58Z

@hubertus65 I've now removed the modifications to the Solar stuff since they will be completely rewritten in #266 and also removed the docx. Btw, it helps when you create your PRs from a topic branch rather than your master-branch since it could well take some time until your PR gets merged and until then your master is kind of locked to a PR.

@GiuseppeLaera

Fine for now but I suggest that @GiuseppeLaera does the final technical review.

OpenIPSL/Electrical/Machines/PSSE/GENSAL.mo

dietmarw · 2021-05-21T13:59:31Z

I think it would be OK to include this for the 2.0.0 release

Added Boolean variables y2l0 and y3l0 and wrapped pre() around them in when-clasue condition. Result differs slightly for x2, m, y2, y3 after time = 300s. and looks more "logical" to me than the previous result in Dymola (Note: orginal version is rejected in OCT). Will attach plots in PR.

Changes all instances for initializing complex numbers from a + jb to Complex(a,b). This makes results look visually identical to the Dymla results, and corrects 2 sign-errors that were present in the zip-file attached to OpenIPSL#263.

changed to Boolean tc = m > 1.0 or m < 1.0, which makes it leagal Modelica. Coorresponding test model now works in OCT

Syntax changes from a + j*b to Complex(a,b). All Examples.Machines.PSSE models run in OCT and gie visually the same results as Dymola.

Accidental commit.

Co-authored-by: Marcelo de C. Fernandes <decasm3@rpi.edu>

github-actions · 2022-06-09T08:08:33Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

dietmarw · 2022-06-09T08:12:01Z

I've rebased this PR now. @hubertus65 just a heads up that you did this PR based on your master branch and not on a topic branch. This is not ideal in case you want to work on something else from the OpenIPSL. So your master branch is basically now locked to this PR until it is finally merged.
@hubertus65 @GiuseppeLaera you both need still to accept the CLA in order that this PR can get merged.

hubertus65 · 2022-06-09T12:27:13Z

I have read the CLA Document and I hereby sign the CLA

hubertus65 · 2022-06-09T12:28:17Z

I hope that did it for signing? FWIW: I'd like to sign the entity CLA for Modelon (I have read both), and am not sure if this fixes even that.

GiuseppeLaera · 2022-06-09T12:43:57Z

I have read the CLA Document and I hereby sign the CLA

dietmarw · 2022-06-09T12:46:50Z

I hope that did it for signing? FWIW: I'd like to sign the entity CLA for Modelon (I have read both), and am not sure if this fixes even that.

Yes CLA check is passed now. Not sure what you mean with CLA for Modelon here.

marcelofcastro · 2022-06-09T14:01:31Z

I hope that did it for signing? FWIW: I'd like to sign the entity CLA for Modelon (I have read both), and am not sure if this fixes even that.

Yes CLA check is passed now. Not sure what you mean with CLA for Modelon here.

@dietmarw I think he meant the CLA-entity which is different from the individual CLA. This is because the CLA for an entity (Modelon in this case) helps people that act on the behalf of a legal entity that cannot be considered a person.

So I guess @hubertus65 was interested in signing the entity CLA. I believe that, for what we have now, if you sign the CLA, you will be signing the appropriate one for your case. In this case, since you mentioned it here in this PR, you are signing both the individual and the entity one and that sentence you wrote above will be already enough. Does that work for you, @hubertus65 ?

marcelofcastro · 2022-06-09T14:05:18Z

The CLA instructions may help you with that @hubertus65 and @dietmarw. I think @hubertus65's concerns are clarified in item 3.

hubertus65 · 2022-06-09T15:37:37Z

I meant to sign both the individual and the entity CLA. If other's from Modelon with a modelon.com email address commit in the future, they should be covered and can sign the individual CLA. That works for me. regarding this PR: I think that @GiuseppeLaera has this plus a lot of other fixes on this side, so I think it is better if he comes with a PR that has the full Monty of compatibility changes.

dietmarw

I got finally around to approve and merge this for the upcoming release.

hubertus65 changed the title ~~Changes to ULTC~~ Changes to ULTC and this time correct syntax fixes to make Examples.Machines.PSSE.GENROE work in OCT Dec 30, 2020

hubertus65 mentioned this pull request Dec 30, 2020

OCT Compliance #263

Closed

3 tasks

hubertus65 changed the title ~~Changes to ULTC and this time correct syntax fixes to make Examples.Machines.PSSE.GENROE work in OCT~~ Changes to ULTC and this time correct syntax fixes to make Examples.Machines.PSSE.GENROE work in OCT & Additional syntax fixes for OCT. Dec 31, 2020

dietmarw previously requested changes Jan 4, 2021

View reviewed changes

lvanfretti assigned GiuseppeLaera Jan 4, 2021

lvanfretti added the octcompliance label Jan 4, 2021

lvanfretti added this to the 2.1 milestone Jan 4, 2021

lvanfretti linked an issue Jan 4, 2021 that may be closed by this pull request

OCT Compliance #263

Closed

3 tasks

tinrabuzin mentioned this pull request Jan 9, 2021

Clean-up and reimplementation of PowerFactory solar models #266

Merged

marcelofcastro requested a review from GiuseppeLaera February 4, 2021 14:34

marcelofcastro reviewed May 4, 2021

View reviewed changes

OpenIPSL/Electrical/Machines/PSSE/GENSAL.mo Outdated Show resolved Hide resolved

dietmarw force-pushed the master branch from 8654bf0 to 7dffe5b Compare May 21, 2021 13:55

Hubertus Tummescheit and others added 11 commits June 9, 2022 10:05

Minor change to make tc legal Modelica code

86946f3

changed to Boolean tc = m > 1.0 or m < 1.0, which makes it leagal Modelica. Coorresponding test model now works in OCT

Syntax changes to make work with OCT

9b63527

Syntax changes from a + j*b to Complex(a,b). All Examples.Machines.PSSE models run in OCT and gie visually the same results as Dymola.

replaced comparison with real with proper numerical method.

e64c7c2

Fixed numerical issue with comparison to Real

9392ef9

Delete WorksInOCT.mo

003ad53

Accidental commit.

Update ULTC.mo

fe011f2

Remove docx which was added (probably accidentally)

b709537

Fix Park transformation GENSAL

513ddc0

Co-authored-by: Marcelo de C. Fernandes <decasm3@rpi.edu>

Conversion to MSL 4.0.0

0c0bb1b

dietmarw force-pushed the master branch from 7dffe5b to 0c0bb1b Compare June 9, 2022 08:08

Whitespace clean up

547fee3

github-actions bot added a commit that referenced this pull request Jun 9, 2022

hubertus65 has signed the CLA in #265

1121368

github-actions bot added a commit that referenced this pull request Jun 9, 2022

GiuseppeLaera has signed the CLA in #265

d97140c

lvanfretti mentioned this pull request Aug 16, 2022

Invalid relations on reals #308

Closed

dietmarw removed the request for review from GiuseppeLaera January 12, 2023 15:00

dietmarw assigned dietmarw and unassigned GiuseppeLaera Jan 12, 2023

dietmarw approved these changes Jan 12, 2023

View reviewed changes

Merge branch 'master' into master

aa4d6d9

dietmarw changed the title ~~Changes to ULTC and this time correct syntax fixes to make Examples.Machines.PSSE.GENROE work in OCT & Additional syntax fixes for OCT.~~ Changes to ULTC and correct syntax fixes for OCT. Jan 12, 2023

dietmarw changed the title ~~Changes to ULTC and correct syntax fixes for OCT.~~ Changes to ULTC and correct syntax fixes for OCT Jan 12, 2023

dietmarw merged commit 11f2ca2 into OpenIPSL:master Jan 12, 2023

github-actions bot locked and limited conversation to collaborators Jan 12, 2023

dietmarw added Tool specific Tool specific issues performance and removed octcompliance labels Jan 29, 2026

dietmarw changed the title ~~Changes to ULTC and correct syntax fixes for OCT~~ perf: Changes to ULTC and correct syntax fixes for OCT Jan 29, 2026

Conversation

hubertus65 commented Dec 29, 2020

Uh oh!

hubertus65 commented Dec 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lvanfretti Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dietmarw commented Jan 28, 2021

Uh oh!

Uh oh!

dietmarw commented May 21, 2021

Uh oh!

github-actions bot commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dietmarw commented Jun 9, 2022

Uh oh!

hubertus65 commented Jun 9, 2022

Uh oh!

hubertus65 commented Jun 9, 2022

Uh oh!

GiuseppeLaera commented Jun 9, 2022

Uh oh!

dietmarw commented Jun 9, 2022

Uh oh!

marcelofcastro commented Jun 9, 2022

Uh oh!

marcelofcastro commented Jun 9, 2022

Uh oh!

hubertus65 commented Jun 9, 2022

Uh oh!

dietmarw left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lvanfretti Jan 4, 2021 •

edited

Loading

github-actions bot commented Jun 9, 2022 •

edited

Loading