#3765 closed defect (fixed)
Bad macro for semiLinear
Reported by: | 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)
Change History (13)
by , 9 years ago
Attachment: | semiLinear_bug.mo added |
---|
comment:1 by , 9 years ago
comment:3 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:4 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Looks like I was sloppy. The cpp version also needs it in:
OMCompiler/SimulationRuntime/cpp/Include/Core/Math/Functions.h
comment:6 by , 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.
follow-up: 10 comment:7 by , 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 , 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 , 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.
comment:10 by , 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 , 9 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:12 by , 7 years ago
Milestone: | Future → 1.11.0 |
---|
Thank you for the analysis!