Opened 7 years ago
Closed 7 years ago
#4939 closed defect (fixed)
mod(x,y) with x < 0 computed incorrectly in algorithms
Reported by: | Francesco Casella | Owned by: | Martin Sjölund |
---|---|---|---|
Priority: | high | Milestone: | 1.13.0 |
Component: | Code Generation | Version: | |
Keywords: | Cc: | Martin Sjölund |
Description
Please run this simple test case (also attached)
model TestMod Integer j, j_mod; Integer k, k_mod; algorithm when initial() then j := -5; j_mod := mod(j,3); end when; equation k = -5; k_mod = mod(k,3); end TestMod;
k_mod
is correcly computed in the equation section as 1, while j_mod
is incorrectly computed in the algorithm section as -2. This is a bit weird, as the same Modelica function is used in both cases.
@lochel, could you have a quick look if this is an easy fix?
Attachments (1)
Change History (11)
by , 7 years ago
Attachment: | TestMod.mo added |
---|
comment:1 by , 7 years ago
Summary: | mod() with negative argument works incorrectly in algorithms → mod(x,y) with x < 0 computed incorrectly in algorithms |
---|
comment:2 by , 7 years ago
comment:3 by , 7 years ago
The difference is when the mod
function is evaluated. It works correctly if it is evaluated during compile time. The runtime implementation is wrong. Is fixed it in the generated c code but now I have to check where this code is actually generated.
comment:6 by , 7 years ago
The bootstrapped compiler depends on generating the wrong code for integer modulus, so it's not that easy to fix this.
follow-up: 8 comment:7 by , 7 years ago
So the specification says to use:
return x-floor(x/y)*y;
Which works fine for doubles. But for integers, this fails if the integer is so large that it does not fit in a double precision float.
The old code had:
return x%y;
Which returns negative values in some cases. I think the correct code would then be:
tmp = x%y; return tmp < 0 ? tmp+y : tmp;
comment:8 by , 7 years ago
Replying to sjoelund.se:
Which returns negative values in some cases. I think the correct code would then be:
tmp = x%y; return tmp < 0 ? tmp+y : tmp;
That should work just well.
comment:9 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:10 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
This is a problem with the generated code for integer mod(). It is missing a floor() call. It is slightly problematic that Ceval and Simplify do not simply call mod(), and instead have their own definition without this problem so I will fix the generated code and these modules.