Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#5209 closed defect (fixed)

Non-evaluated package constants and issue with apparently unused functions with the NF

Reported by: casella Owned by: perost
Priority: high Milestone: 2.0.0
Component: New Instantiation Version:
Keywords: Cc:

Description (last modified by casella)

Please check Modelica.Electrical.Spice3.Examples.Graetz. The C compiler complains about

Modelica_trunk_Modelica.Electrical.Spice3.Examples.Graetz_functions.c:1812:24: error: use of undeclared identifier '_Modelica'
  _Kelvin = _Celsius - _Modelica._Constants._T_zero;
                       ^

The output of the NF contains

function Modelica.SI.Conversions.from_degC "Inline before index reduction" "Convert from degCelsius to Kelvin"
  input Real Celsius(quantity = "ThermodynamicTemperature", unit = "degC") "Celsius value";
  output Real Kelvin(quantity = "ThermodynamicTemperature", unit = "K", displayUnit = "degC", min = 0.0, start = 288.15, nominal = 300.0) "Kelvin value";
algorithm
  Kelvin := Celsius - Modelica.Constants.T_zero;
end Modelica.SI.Conversions.from_degC;

where, for some reason, Modelica.Constants.T_zero was not replaced with -273.15, as the old front-end does. I guess that should be fixed.

There are two additional weird things in the way this model is flattened.

One is that the function from_degC is apparently not called anywhere - I couldn't find any more instance of that function name in the flattened code besides the function definition itself. So the first question is

  • why is this function definition included in the flat model in the first place?

BTW, this happens also with the old front-end.

The other weird thing is that the following declaration shows up in the body of the Graetz model

  final constant Real Modelica.Constants.T_zero = -273.15 "Absolute zero temperature";

while T_zero is not used anywhere in that model - it is only used in the from_degC function body. So, two additional questions arise

  • why is this constant defined here and not in the function body?
  • why aren't all constants defined by literal values constant-evaluated by the front-end in the first place?

This issue may affect other models in other libraries, as it appears to be at a rather fundamental level.

Change History (5)

comment:1 Changed 6 years ago by casella

  • Description modified (diff)

comment:2 follow-up: Changed 6 years ago by perost

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in 4cccc38.

The issue was that the evaluation of constants in functions was done separately from collecting used functions, and that phase didn't process record constructors. The SpiceConstants record had the field:

constant Real CONSTvt0 = Modelica.Constants.k * SI.Conversions.from_degC(27) / CHARGE;

The constructor was skipped by the constant evalution phase, then collected during the collection phase which also collected the from_degC function, and finally it was simplified which caused the from_degC call to be evaluated and disappear.

Now I just evaluate and simplify functions before collecting them, so from_degC will now be optimized away and not collected at all. I don't know why I didn't do that before, hopefully we won't find out.

The old frontend generally collects many more functions, I think it's collecting them before doing any optimizations. The new frontend should only collect functions that are used in the flat model, with the exception of record constructors that are sometimes needed by the backend even if they're not explictly called.

As for your questions:

why is this constant defined here and not in the function body?

This is a remainder from early on when we tried to add package constants to the model instead of evaluating them. But that didn't work out, so now we just evaluate them instead. But I kept the phase that collects package constants and adds them to the model, since it takes negligible time and is useful since you can just look at the top of a flat model and see if it has any issues with constants that should've been evaluated but wasn't.

why aren't all constants defined by literal values constant-evaluated by the front-end in the first place?

Because bugs.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 6 years ago by casella

Replying to perost:

Fixed in 4cccc38.

Excellent!

why is this constant defined here and not in the function body?

This is a remainder from early on when we tried to add package constants to the model instead of evaluating them. But that didn't work out, so now we just evaluate them instead. But I kept the phase that collects package constants and adds them to the model, since it takes negligible time and is useful since you can just look at the top of a flat model and see if it has any issues with constants that should've been evaluated but wasn't.

Sounds good in this case. However, there is another use case which could be problematic, which is found in the ExternalMedia library. There, we have some package constants whose value is computed by an external function call. The external function call is actually quite time consuming, so it is essential that this function call is only performed once at initialization and then no more.

I wonder how the NF can handle this case - would it try to evaluate the external function during flattening (which is not trivial to handle), or would it replace each instance of the package constant with the function call? In this second case, I guess we should rely on CSE by the back-end to make sure the function is actually only called once at initialization, and then the result kept as a constant during simulation.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 6 years ago by perost

Replying to casella:

Replying to perost:

Fixed in 4cccc38.

Excellent!

why is this constant defined here and not in the function body?

This is a remainder from early on when we tried to add package constants to the model instead of evaluating them. But that didn't work out, so now we just evaluate them instead. But I kept the phase that collects package constants and adds them to the model, since it takes negligible time and is useful since you can just look at the top of a flat model and see if it has any issues with constants that should've been evaluated but wasn't.

Sounds good in this case. However, there is another use case which could be problematic, which is found in the ExternalMedia library. There, we have some package constants whose value is computed by an external function call. The external function call is actually quite time consuming, so it is essential that this function call is only performed once at initialization and then no more.

I wonder how the NF can handle this case - would it try to evaluate the external function during flattening (which is not trivial to handle), or would it replace each instance of the package constant with the function call? In this second case, I guess we should rely on CSE by the back-end to make sure the function is actually only called once at initialization, and then the result kept as a constant during simulation.

Currently it would likely just fail, because we haven't yet implemented evaluation of external function calls in it. How that should be handled is still an open question. In some cases we really do need to evaluate such functions, for example if they're used to determine array dimensions (like in ReadRealMatrixFromFile. There might be some cases where it makes sense to avoid evaluating such calls though.

comment:5 in reply to: ↑ 4 Changed 6 years ago by casella

Replying to perost:

Currently it would likely just fail, because we haven't yet implemented evaluation of external function calls in it. How that should be handled is still an open question. In some cases we really do need to evaluate such functions, for example if they're used to determine array dimensions (like in ReadRealMatrixFromFile. There might be some cases where it makes sense to avoid evaluating such calls though.

In this case those package constants are not structural parameters, so there is no need to evaluate them in the front-end, as long as the back-end can figure out they need to be computed only once. I'll get back to you with this when I pick up the work on ExternalMedia in the next few weeks.

Note: See TracTickets for help on using tickets.