Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#3765 closed defect (fixed)

Bad macro for semiLinear

Reported by: crupp@… Owned by: somebody
Priority: blocker Milestone: 1.11.0
Component: Run-time Version: v1.10.0-dev-nightly
Keywords: Cc: Rüdiger Franke, Niklas Worschech

Description

In openmodelica.h the macro for semiLinear is defined as:
#define semiLinear(x,positiveSlope,negativeSlope) (x>=0?positiveSlope*x:negativeSlope*x)

If x is an expression such as x=-1+a/b then the macro evaluates it incorrectly.

It should be:
#define semiLinear(x,positiveSlope,negativeSlope) (x>=0?positiveSlope*(x):negativeSlope*(x))

The added parentheses fix the issue. I've attached a test case that illustrates the issue. In this test the expression is transformed into semiLinear(-1+d/L,k,0).

It turns out that this is a long-standing issue since it was implemented (2010). Maybe it will magically fix some test cases.

Attachments (1)

semiLinear_bug.mo (214 bytes ) - added by crupp@… 9 years ago.

Download all attachments as: .zip

Change History (13)

by crupp@…, 9 years ago

Attachment: semiLinear_bug.mo added

comment:1 by Vitalij Ruge, 9 years ago

Thank you for the analysis!

comment:2 by Vitalij Ruge, 9 years ago

your fixes was added with PR528

comment:3 by Vitalij Ruge, 9 years ago

Resolution: fixed
Status: newclosed

comment:4 by crupp@…, 9 years ago

Resolution: fixed
Status: closedreopened

Looks like I was sloppy. The cpp version also needs it in:
OMCompiler/SimulationRuntime/cpp/Include/Core/Math/Functions.h

comment:5 by Adrian Pop, 9 years ago

Cc: Rüdiger Franke Niklas Worschech added

Added rfranke and niklwors to CC for the CPP runtime.

comment:6 by crupp@…, 9 years ago

Also, vitalij was smart and caught the error in my fix, it should be:
#define semiLinear(x,positiveSlope,negativeSlope) (x>=0?(positiveSlope)*(x):(negativeSlope)*(x))

Is there any reason that the conditional x>=0 should be (x)>=0? The only reason I can think of is if there was some sort of bitwise operator. I don't know how that would get there, but it may be better to be safe than sorry.

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

I think I would have preferred to make it a static inline function, or just inline it in the code generator for the simple reason that x is present in multiple places and could in some instances cause unexpected behaviour.

comment:8 by Rüdiger Franke, 9 years ago

@crupp: can you be a bit more specific what you mean with "Looks like I was sloppy. The cpp version also needs it in: OMCompiler/SimulationRuntime/cpp/Include/Core/Math/Functions.h"?

The inline function should be ok:

inline static double semiLinear(double x,double positiveSlope,double negativeSlope)
{
 if(x>=0)
    return positiveSlope*x;
 else
    return negativeSlope*x;
}

comment:9 by crupp@…, 9 years ago

Having not used them much in the past, I was assuming that inlined functions are treated similarly to macros in that it mixes the operations. After reading into it, it looks like I'm wrong and that it still behaves like a function. So, I retract my statement. Sorry for the trouble.

in reply to:  7 comment:10 by Vitalij Ruge, 9 years ago

Replying to sjoelund.se:

I think I would have preferred to make it a static inline function, or just inline it in the code generator for the simple reason that x is present in multiple places and could in some instances cause unexpected behaviour.

I like the idea with inline function. PR 533

principally it work for DIVSION_SIM PR 534, too??

comment:11 by Vitalij Ruge, 9 years ago

Resolution: fixed
Status: reopenedclosed

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

Milestone: Future1.11.0
Note: See TracTickets for help on using tickets.