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. I 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.