Opened 6 years ago
Closed 6 years ago
#4939 closed defect (fixed)
mod(x,y) with x < 0 computed incorrectly in algorithms
Reported by: | casella | Owned by: | sjoelund.se |
---|---|---|---|
Priority: | high | Milestone: | 1.13.0 |
Component: | Code Generation | Version: | |
Keywords: | Cc: | sjoelund.se |
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)
Changed 6 years ago by casella
comment:1 Changed 6 years ago by casella
- Summary changed from mod() with negative argument works incorrectly in algorithms to mod(x,y) with x < 0 computed incorrectly in algorithms
comment:2 Changed 6 years ago by sjoelund.se
comment:3 Changed 6 years ago by lochel
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:4 Changed 6 years ago by lochel
It seems Martin was faster...
comment:5 Changed 6 years ago by sjoelund.se
comment:6 Changed 6 years ago by sjoelund.se
The bootstrapped compiler depends on generating the wrong code for integer modulus, so it's not that easy to fix this.
comment:7 follow-up: ↓ 8 Changed 6 years ago by sjoelund.se
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 in reply to: ↑ 7 Changed 6 years ago by lochel
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 Changed 6 years ago by sjoelund.se
- Owner changed from lochel to sjoelund.se
- Status changed from new to assigned
comment:10 Changed 6 years ago by sjoelund.se
- Resolution set to fixed
- Status changed from assigned to 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.