Skip to content

MML4 draft #537

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

MML4 draft #537

wants to merge 16 commits into from

Conversation

edwing-met
Copy link
Contributor

@edwing-met edwing-met commented Jun 27, 2025

Goal

PR adding connectors to most CCGT and NPP components
A lot of parameters have also been given a default value inside the component so there is no need to attribute it in the model

Type of change

  • Bugfix
  • New feature
  • Refactoring change
  • Release & Version Update (don't forget to change the version number in package.mo)

Will it break anything in previous models ?

  • Breaking change (If yes, make sure to point it out in the changelog)
  • Non-Breaking change

Checklist

  • I have added the appropriate tags, reviewers, projects (and detailed the size and priority of my PR) and linked issues to this PR
  • I have performed a self-review of my own code
  • I have checked that all existing tests pass
  • I have checked that my work is compatible with OpenModelica
  • I have added/updated tests that prove my development works and does not break anything.
  • I have made corresponding changes or additions to the documentation (in Notion documentation)
  • I have added corresponding entries to the Changelog
  • I have checked for conflicts with target branch, and merged/rebased in consequence

You can also fill these out after creating the PR, but make sure to check them all before submitting your PR for review.

@edwing-met edwing-met changed the title MML4 MML4 draft Jun 27, 2025
@edwing-met edwing-met added the ✨enhancement New feature or request label Jun 27, 2025
Copy link
Contributor

@hadrienp-met hadrienp-met left a comment

Choose a reason for hiding this comment

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

Merci ! Deux remarques principales :

  • Il y a peut être un truc qui m'échappe mais je comprends pas pourquoi on a besoin de définir des paramètres _constant ? Tel qu'ils sont utilisés, ce sont des variables d'initalisation mais on n'en a pas besoin por les paramètres
  • Il reste des composants à adapter comme l'ACC, je vais faire la liste
  • Idéalement il faudrait qu'on ai un modèle direct aussi de Metroscopia
  • Et il faudra adapter tous les tests
    Bref pas mal de boulot qui doit mobiliser toute l'équipe !

Comment on lines +15 to +16
parameter Real tau_constant = 15;
parameter Real eta_is_constant = 0.8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Pourquoi on a ces paramètres ? Ils ne sont pas utilisés ensuite

@@ -12,9 +12,9 @@ model GasTurbine
import MetroscopeModelingLibrary.Utilities.Units;
import MetroscopeModelingLibrary.Utilities.Units.Inputs;

parameter Units.Yield eta_is_constant = 0.8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem pourquoi ce paramètre ?

Suggested change
parameter Units.Yield eta_is_constant = 0.8;

Copy link
Contributor

Choose a reason for hiding this comment

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

A supprimer

Inputs.InputFrictionCoefficient Kfr_hot(start=0);
Inputs.InputArea S;
parameter Units.Area S = 15000;
parameter Units.MassFraction x_steam_out = 1; // Steam mass fraction at water outlet
Copy link
Contributor

Choose a reason for hiding this comment

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

Il faut garder les surfaces en paramètres modifiable, on l'utilise dans certains cas

Inputs.InputHeatExchangeCoefficient Kth;
Inputs.InputFrictionCoefficient Kfr_cold;
Inputs.InputFrictionCoefficient Kfr_hot;
parameter Units.Area S = 3000;
Copy link
Contributor

Choose a reason for hiding this comment

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

On modifie parfois les surfaces pour faire des hypotheses

@@ -26,6 +26,13 @@ model GasTurbine
Units.Percentage eta_is_decrease(min = 0, max=100) "percentage decrease of eta_is";

Power.Connectors.Outlet C_W_shaft annotation (Placement(transformation(extent={{90,90},{110,110}}), iconTransformation(extent={{90,90},{110,110}})));
Utilities.Interfaces.GenericReal eta_is(start=eta_is_constant) annotation (Placement(transformation(
Copy link
Contributor

Choose a reason for hiding this comment

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

On a un besoin d'initialiser les eta is ? ça me semble pas utile

@@ -5,7 +5,7 @@ partial model ControlValve
import MetroscopeModelingLibrary.Utilities.Units.Inputs;
import MetroscopeModelingLibrary.Utilities.Constants;

Inputs.InputCv Cv_max(start=1e4) "Maximum CV";
parameter Units.Cv Cv_max_constant = 1e4 "Maximum CV";
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem pourquoi le constant ?

parameter Boolean faulty = false;
Units.Percentage fouling; // Fouling coefficient

parameter Units.Height delta_z_constant = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

constant ?


parameter Boolean faulty = false;
Units.Percentage closed_valve; // Valve not fully opened

parameter Real Cv_constant = 1e4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants