#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 )
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 , 7 years ago
Description: | modified (diff) |
---|
comment:2 by , 7 years ago
comment:3 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:4 by , 7 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 , 7 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:7 by , 6 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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.
follow-up: 9 comment:8 by , 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.
follow-up: 10 comment:9 by , 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.
follow-up: 11 comment:10 by , 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.
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. But since there is already a "tradition" on this (i.e. MS considers and tools and users are aware of this) my concerns are somewhat outdated.
follow-up: 13 comment:11 by , 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 , 6 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Fixed in 7d0153e3. The issue was breaking the rule: "Thou shalt not simplify expressions before unrolling loops".
comment:13 by , 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 , 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.
follow-up: 16 comment:15 by , 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?
comment:16 by , 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.
The problem is that the NF says that the function is
sum<metatype>
instead ofsum<Real>
.