Skip to content

Feat/empirical turbine#83

Open
boerrebj wants to merge 41 commits into
OpenSimHub:masterfrom
boerrebj:feat/empiricalTurbine
Open

Feat/empirical turbine#83
boerrebj wants to merge 41 commits into
OpenSimHub:masterfrom
boerrebj:feat/empiricalTurbine

Conversation

@boerrebj

@boerrebj boerrebj commented Mar 3, 2026

Copy link
Copy Markdown
Collaborator

Turbine model based on emprirical relations for reaction turbines (includes throtling effect).

@boerrebj
boerrebj requested a review from dietmarw March 3, 2026 14:46
@github-actions github-actions Bot added the feat label Mar 3, 2026
@dietmarw
dietmarw force-pushed the feat/empiricalTurbine branch from 1425890 to cf83711 Compare March 3, 2026 15:03
@dietmarw

dietmarw commented Mar 3, 2026

Copy link
Copy Markdown
Collaborator

@boerrebj I've run the test models with Dymola and got non-linear system warnings/errors but Dymola manages to complete the simulation. The diagnostics are slightly different when running Dymola 2020 to Dymola 2026x , So I'm including both here in case something rings a bell:
Dymola2020.txt
Dymola2026x.txt

@dietmarw
dietmarw marked this pull request as draft March 3, 2026 17:10
@boerrebj
boerrebj marked this pull request as ready for review April 21, 2026 12:27
@boerrebj

Copy link
Copy Markdown
Collaborator Author

Should be ready for review and merge with main trunk.

@boerrebj
boerrebj removed the request for review from dietmarw June 25, 2026 08:05
@boerrebj
boerrebj requested a review from dietmarw July 7, 2026 08:30

@dietmarw dietmarw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've added a couple of commits for cleanup purposes. After that, there are a couple of things like to discuss before merging:

  1. OpenHPLTest.TorqueEquation: This package contains (I think) meaningless tests. The models themselves are already used by the other base classes and are used in real examples. So I guess we can safely remove that package. So you agree?
  2. EmpiricalTurbine:
    • The model contains an unused constant eps, remove?
    • The constants alpha and beta have no description and no type so it would be hard to know what they stand for.
    • (already fixed) uncecessarily assignment of rho, just use data.rho directly, same with g
  3. OpenHPLTest.Empirical:
  • I've simplified the Test 04 and 05 models to extend from the sam 01 model like the others. This is when I noticed that 04 and 05 are actually identical. So should vi remove that?
  • I've also simplified Test06 and 07 with help of extension.
    .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants