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)

TestMod.mo (203 bytes ) - added by Francesco Casella 7 years ago.

Download all attachments as: .zip

Change History (11)

by Francesco Casella, 7 years ago

Attachment: TestMod.mo added

comment:1 by Francesco Casella, 7 years ago

Summary: mod() with negative argument works incorrectly in algorithmsmod(x,y) with x < 0 computed incorrectly in algorithms

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

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.

comment:3 by Lennart Ochel, 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.

Version 0, edited 7 years ago by Lennart Ochel (next)

comment:4 by Lennart Ochel, 7 years ago

It seems Martin was faster...

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

The bootstrapped compiler depends on generating the wrong code for integer modulus, so it's not that easy to fix this.

comment:7 by Martin Sjölund, 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;

in reply to:  7 comment:8 by Lennart Ochel, 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 Martin Sjölund, 7 years ago

Owner: changed from Lennart Ochel to Martin Sjölund
Status: newassigned

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

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.