modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
471 stars 168 forks source link

Overriding final parameters #3970

Open HansOlsson opened 2 years ago

HansOlsson commented 2 years ago

Reverting #1783 change implies that we have a number of final parameters that are part of records where the entire record gets a new value.

In most cases it's the same value (since the new record is using the same record class), and it could be that we should refine the specification to handle that better; that's a separate issue.

However, in Modelica.Magnetic.FundamentalWave.Examples.BasicMachines.InductionMachines.ComparisonPolyphase.IMC_DOL_Polyphase the value of aimcM.statorCoreParameters.m is 3 - which is unusual for a 5-phase machine.

There are at least 4 models with this mismatch in MSL, all in ComparisonPolyphase packages specifically:

Since they are intended for comparison it doesn't seem that critical - it more means that one variant works and the other does not.

HansOlsson commented 2 years ago

@AHaumer @christiankral any comment?

christiankral commented 1 year ago

On my opinion this is not solely an example issue as the current implementation (being in line with the MLS) does not allow to parameterize the polyphase machine core loss parameters for m > 3 based on the record Modelica.Electrical.Machines.Utilities.ParameterRecords.InductionMachineData. Even though it was the original intention to allow such parameterization for m > 3 it obviously does not work.

The reason for m=3 appearing statoreCoreParameters is that the general induction machine parameter Modelica.Electrical.Machines.Utilities.ParameterRecords.InductionMachineData shows the following code:

  parameter Integer m=3 "Number of phases" annotation(Evaluate=true);
  ...
  parameter Machines.Losses.CoreParameters statorCoreParameters(
    final m=m,
    PRef=0,
    VRef=100,
    wRef=2*pi*fsNominal)
    "Stator core loss parameter record; all parameters refer to stator side"
    annotation (Dialog(tab="Losses"));
  ...

The propagation of the parameter m = 3 to the record statorCoreParameters by means of final m=m kind of fixes m = 3: so this is the problem.

Up to now I have only one solution to this issue.

image

christiankral commented 1 year ago

My propsoal also requires to change the actual implementation of

  parameter Machines.Losses.CoreParameters rotorCoreParameters(
    final m=3,
    PRef=0,
    VRef=1,
    wRef=1)
    "Rotor core loss parameter record; all parameters refer to rotor side"
    annotation (Dialog(tab="Losses"));

to m = m without the final attribute in the paramerer reocrd Modelica.Electrical.Machines.Utilities.ParameterRecords.IM_SlipRingData

henrikt-ma commented 1 year ago

This is a backwards compatible change, since m = m is "weaker" than final m = m

Maybe it's backwards compatible, but being "weaker" is not a sufficient condition to ensure this. To be sure, one needs to also check that the m without non-final modification can be clearly classified as either a parameter not to be evaluated, or a parameter to be evaluated. Otherwise, removing final could break propagation of constant evaluation, so that resulting simulation models have unintentional parametric complexity.

(While I know it isn't a popular idea in our community, a remedy to this is often to declare the variability as constant instead; then one doesn't need final to ensure that a constant value of the modifier expression is propagated to the modified property.)

HansOlsson commented 1 year ago

(While I know it isn't a popular idea in our community, a remedy to this is often to declare the variability as constant instead; then one doesn't need final to ensure that a constant value of the modifier expression is propagated to the modified property.)

Changing it to a constant does not guarantee that it is backward compatible.

(I will review the actual change later.)