Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#4864 closed defect (fixed)

The NF does not expand the sum() function

Reported by: Francesco Casella Owned by: Per Östlund
Priority: high Milestone: 2.0.0
Component: New Instantiation Version:
Keywords: Cc:

Description (last modified by Francesco Casella)

Please check the ScalableTestSuite.Thermal.HeatExchanger.ScaledExperiments.CocurrentHeatExchangerEquations_N_10 model. It fails at link time due to the lack of definition of the sum_metatype_array type.

The NF flattens this equation

QtotA = sum(QA);

while the old FE gives

  QtotA = QA[1] + QA[2] + QA[3] + QA[4] + QA[5] + QA[6] + QA[7] + QA[8] + QA[9];

As in #4863 I'm not sure if we should expand the array when the DAE structure is created, or rather extend the code generation to handle the sum function explicitly.

If expanding the array in the NF is easily accomplished, I would suggest to do that, at least for the time being, so we can get the maximum coverage without waiting for somebody to fix the code generation.

Change History (16)

comment:1 by Francesco Casella, 7 years ago

Description: modified (diff)

comment:2 by Martin Sjölund, 7 years ago

The problem is that the NF says that the function is sum<metatype> instead of sum<Real>.

comment:3 by Francesco Casella, 7 years ago

Resolution: fixed
Status: newclosed

This issue was fixed in #2372, see report

comment:4 by Francesco Casella, 7 years ago

Resolution: fixed
Status: closedreopened

In fact, there are still cases where the sum() function is not expanded, e.g. Modelica.Electrical.MultiPhase.Examples.Rectifier, where the old FE gives

star1.plug_p.pin[1].i + star1.plug_p.pin[2].i + star1.plug_p.pin[3].i + star1.pin_n.i = 0.0;

while the NF gives

sum(star1.plug_p.pin.i) + star1.pin_n.i = 0.0;

In this case, this issue causes the failure of the matching algorithm.

comment:5 by Francesco Casella, 6 years ago

Another probably related issue is found in Modelica.Electrical.PowerConverters.Examples.ACDC.RectifierBridge2mPulse.DiodeBridge2mPulse. The following error is reported

Error: An independent subset of the model has imbalanced number of equations (237)
and variables (236).

Running the same model in OMEdit gives:

[3] 01:34:04 Symbolic Error
[Modelica.Electrical.MultiPhase: 694:7-694:38]:
Found equation without time-dependent variables:
currentSensor.i = -sum(rectifier.star_p.plug_p.pin.i)

[4] 01:34:04 Symbolic Error
An independent subset of the model has imbalanced number of equations (237)
and variables (236).

so it seems that unexpanded sum() is the root cause here as well.

comment:6 by Francesco Casella, 6 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in bc961b4f7.

comment:7 by Francesco Casella, 6 years ago

Resolution: fixed
Status: closedreopened

In fact, there is still a remaining glitch, see e.g. ThermoPower.Test.DistributedParameterComponents.TestWaterFlow1DFV_DB, which now fails because of

[OMCompiler/Compiler/BackEnd/Differentiate.mo:402:3-402:155:writable] Error:
Derivative of expression "hexFV.wbar[1] = hexFV.w - sum(hexFV.dMdt[1:0]) - 0.5 * hexFV.dMdt[1]"
w.r.t. "dummyVarLSJac2" is non-existent.

I guess the sum of an empty vector, such as sum(hexFV.dMdt[1:0]), should just be removed from the expression.

comment:8 by massimo ceraolo, 6 years ago

If I understand well the context (I did not check the model mentioned in comment 7), "remove from the expression" here means attributing zero to it.
I think doing this is incorrect.
The sum of no elements is undetermined, and IMO, using such a sum in an expression should just trigger an error message.

in reply to:  8 ; comment:9 by Per Östlund, 6 years ago

Replying to ceraolo:

The sum of no elements is undetermined, and IMO, using such a sum in an expression should just trigger an error message.

Table 10-8 in the specification says that a sum reduction of an empty array is zeros(...) (the reduction expression might itself be an array). It doesn't actually say what sum(A) where A is an empty array should be though, but I assume that's an oversight and table 10-8 should apply to all forms of sum. That's at least how both OpenModelica and Dymola implements it, and it would be weird if sum would behave differently from an equivalent sum reduction.

For the record, the NF does evaluate sum of an empty array to 0, but in this case it seems it fails to expand the argument for some reason.

in reply to:  9 ; comment:10 by massimo ceraolo, 6 years ago

I assume that's an oversight and table 10-8 should apply to all forms of sum.

This is a reasonable consequence. Before commenting I just checked sum(A) in table 10-7, and did not realise that also Table 10-8 could give useful hints.
So I agree that since MS say that sum of an empty expression must be zero, also sum of an empty array should be so.

Version 0, edited 6 years ago by massimo ceraolo (next)

in reply to:  10 ; comment:11 by Francesco Casella, 6 years ago

Replying to ceraolo:

When I wrote my comment 9 I thought of the awful things that may happen when sum() of an empty vector is used as a multiplier, if it is interpreted as zero.

BTW, sum() could be zero anyway, even for non-empty arrays :)

comment:12 by Per Östlund, 6 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in 7d0153e3. The issue was breaking the rule: "Thou shalt not simplify expressions before unrolling loops".

in reply to:  11 comment:13 by massimo ceraolo, 6 years ago

Replying to casella:

Replying to ceraolo:

When I wrote my comment 9 I thought of the awful things that may happen when sum() of an empty vector is used as a multiplier, if it is interpreted as zero.

BTW, sum() could be zero anyway, even for non-empty arrays :)

That's exactly what I meant: a zero due to summing zero-valued elements is totally a different thing from a conventional zero due to a MS rule. Since the MS rule, as I read, has been there for years, and it is used in models and libraries we must accept it. Existing code, evidently, takes care of using sum according to this convention effectively.
People using sum(), for instance as a factor or as a denominator, must be aware that sum() output can be zero not only when zero is the result of a mathematical summation, which is obvious, but also when indeed nothing is summed-up (sum of an empty vector), which is less obvious.

comment:14 by Francesco Casella, 6 years ago

Can you make a concrete example? The only one I can think of is that of a weighted sum:

w = sum(x.*w)/sum(w);

but in this case if the size of w is zero the whole thing is removed from the model outright.

comment:15 by massimo ceraolo, 6 years ago

Indeed I don't have a precise example in mind.

Indeed I can envisage many examples, but in all cases the result is: if the programmer is aware that sum() of an empty array is zero, he can take good countermeasures to avoid crazy results.

In case, instead the programmer writes his equations thinking that his arrays will always have at least one element, and unexpectedly in particular circumstances some array becomes empty, wrong computations (multiplying or adding sum() to something) or domain errors (dividing something by sum()) may occur, causing wrong results or program abortion, without a clue of what the issue is.

But since MS and Dymola and OM are already aware that sum() results to zero when the argument is an empty array, and since existing libraries are aware of this as well, there is no need to discuss more on this, I suppose.

BTW: I imagine that sum(B) when B is empty presently becomes "0" , not "removed".
You say that (I corrected a misprint)

avg = sum(x.*w)/sum(w);

when w is empty is "removed". What does this mean?
What would be the value of avg in that case?
What's the meaning of the ratio of a removed quantity to another removed quantity?

in reply to:  15 comment:16 by Francesco Casella, 6 years ago

Replying to ceraolo:

BTW: I imagine that sum(B) when B is empty presently becomes "0" , not "removed".
You say that (I corrected a misprint)

Sorry, I messed up, of course the left hand-side avg is always defined, whether or not x and w are empty.

What's the meaning of the ratio of a removed quantity to another removed quantity?

If x and w are empty, then avg = 0/0 and you get a division by zero error.

Note: See TracTickets for help on using tickets.