From b2f623e0a2d4f133eb8de115bc37814d2c469f33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20=C3=96stlund?= Date: Wed, 14 Jun 2023 15:16:26 +0200 Subject: [PATCH] Fix handling of function parameter variability (#10836) - Remove variability prefixes from function parameters since they have no semantic meaning, to avoid e.g. function inputs being constant evaluated. - Add some debug output from EvalFunction using the existing evalFuncDump debug flag. Fixes #10828 --- .../Compiler/NFFrontEnd/NFAttributes.mo | 9 +++++ .../Compiler/NFFrontEnd/NFEvalFunction.mo | 8 +++++ OMCompiler/Compiler/NFFrontEnd/NFFunction.mo | 23 ++++++++----- OMCompiler/Compiler/NFFrontEnd/NFInst.mo | 2 +- .../Compiler/NFFrontEnd/NFModelicaBuiltin.mo | 34 +++++++++---------- .../modelica/scodeinst/FuncVariability.mo | 26 -------------- .../scodeinst/FunctionParamVariability.mo | 29 ++++++++++++++++ .../flattening/modelica/scodeinst/Makefile | 2 +- .../modelica/scodeinst/Ticket5821.mo | 4 +-- 9 files changed, 82 insertions(+), 55 deletions(-) delete mode 100644 testsuite/flattening/modelica/scodeinst/FuncVariability.mo create mode 100644 testsuite/flattening/modelica/scodeinst/FunctionParamVariability.mo diff --git a/OMCompiler/Compiler/NFFrontEnd/NFAttributes.mo b/OMCompiler/Compiler/NFFrontEnd/NFAttributes.mo index 584f64d6d9e..1f844510937 100644 --- a/OMCompiler/Compiler/NFFrontEnd/NFAttributes.mo +++ b/OMCompiler/Compiler/NFFrontEnd/NFAttributes.mo @@ -509,6 +509,8 @@ public input output Attributes attr; input Class cls; input InstNode clsNode; + input InstNode compNode; + input InstContext.Type context; protected Variability var = attr.variability; algorithm @@ -516,6 +518,13 @@ public attr := NFAttributes.IMPL_DISCRETE_ATTR; elseif var == Variability.CONTINUOUS and InstNode.isDiscreteClass(clsNode) then attr.variability := Variability.IMPLICITLY_DISCRETE; + elseif var < Variability.CONTINUOUS and InstContext.inFunction(context) and + attr.direction <> Direction.NONE and + isNone(InstNode.getAnnotation("__OpenModelica_functionVariability", compNode)) then + // Variability prefixes on function parameters has no semantic meaning, + // remove them so we don't have to worry about accidentally evaluating + // e.g. an input declared as constant/parameter. + attr.variability := Variability.CONTINUOUS; end if; end updateVariability; diff --git a/OMCompiler/Compiler/NFFrontEnd/NFEvalFunction.mo b/OMCompiler/Compiler/NFFrontEnd/NFEvalFunction.mo index a03eb4ff4a6..bc820ab4a1c 100644 --- a/OMCompiler/Compiler/NFFrontEnd/NFEvalFunction.mo +++ b/OMCompiler/Compiler/NFFrontEnd/NFEvalFunction.mo @@ -139,6 +139,14 @@ algorithm fail(); end try; + if Flags.isSet(Flags.EVAL_FUNC_DUMP) then + print(AbsynUtil.pathString(Function.name(fn)) + " => "); + print(Expression.toString(result)); + print("\nArguments:\n"); + print(UnorderedMap.toString(arg_map, InstNode.name, Expression.toString)); + print("\n"); + end if; + Pointer.update(call_counter, call_count - 1); end evaluateNormal; diff --git a/OMCompiler/Compiler/NFFrontEnd/NFFunction.mo b/OMCompiler/Compiler/NFFrontEnd/NFFunction.mo index e2624f5c064..801afd72c87 100644 --- a/OMCompiler/Compiler/NFFrontEnd/NFFunction.mo +++ b/OMCompiler/Compiler/NFFrontEnd/NFFunction.mo @@ -496,9 +496,10 @@ uniontype Function protected SCode.Element def; Integer numError = Error.getNumErrorMessages(); + InstContext.Type fn_context = InstContext.set(context, NFInstContext.FUNCTION); algorithm try - fnNode := Inst.instantiate(fnNode, context = context, instPartial = true); + fnNode := Inst.instantiate(fnNode, context = fn_context, instPartial = true); else true := Error.getNumErrorMessages() == numError; def := InstNode.definition(fnNode); @@ -1534,11 +1535,14 @@ uniontype Function protected DAE.FunctionAttributes attr; InstNode node = fn.node; + InstContext.Type fn_context; algorithm if not isTyped(fn) then + fn_context := InstContext.set(context, NFInstContext.FUNCTION); + // Type all the components in the function. - Typing.typeClassType(node, NFBinding.EMPTY_BINDING, context, node); - Typing.typeComponents(node, context, preserveDerived = isPartialDerivative(fn)); + Typing.typeClassType(node, NFBinding.EMPTY_BINDING, fn_context, node); + Typing.typeComponents(node, fn_context, preserveDerived = isPartialDerivative(fn)); if InstNode.isPartial(node) then ClassTree.applyComponents(Class.classTree(InstNode.getClass(node)), boxFunctionParameter); @@ -1559,22 +1563,25 @@ uniontype Function protected Boolean pure; DAE.FunctionAttributes attr; + InstContext.Type fn_context; algorithm + fn_context := InstContext.set(context, NFInstContext.FUNCTION); + // Type the bindings of components in the function. for c in fn.inputs loop - Typing.typeComponentBinding(c, context); + Typing.typeComponentBinding(c, fn_context); end for; for c in fn.outputs loop - Typing.typeComponentBinding(c, context); + Typing.typeComponentBinding(c, fn_context); end for; for c in fn.locals loop - Typing.typeComponentBinding(c, context); + Typing.typeComponentBinding(c, fn_context); end for; // Type the algorithm section of the function, if it has one. - Typing.typeFunctionSections(fn.node, context); + Typing.typeFunctionSections(fn.node, fn_context); // Type any derivatives of the function. for fn_der in fn.derivatives loop @@ -1596,7 +1603,7 @@ uniontype Function end if; end if; - if not InstContext.inRelaxed(context) then + if not InstContext.inRelaxed(fn_context) then checkUseBeforeAssign(fn); end if; diff --git a/OMCompiler/Compiler/NFFrontEnd/NFInst.mo b/OMCompiler/Compiler/NFFrontEnd/NFInst.mo index b2aa0107aa8..cf0c9d3470a 100644 --- a/OMCompiler/Compiler/NFFrontEnd/NFInst.mo +++ b/OMCompiler/Compiler/NFFrontEnd/NFInst.mo @@ -1833,7 +1833,7 @@ algorithm checkBindingRestriction(res, binding, node, info); // Update some of the attributes now that we now the type of the component. - ty_attr := Attributes.updateVariability(ty_attr, ty, ty_node); + ty_attr := Attributes.updateVariability(ty_attr, ty, ty_node, node, context); ty_attr := Attributes.updateComponentConnectorType(ty_attr, res, context, node); if not referenceEq(attr, ty_attr) then diff --git a/OMCompiler/Compiler/NFFrontEnd/NFModelicaBuiltin.mo b/OMCompiler/Compiler/NFFrontEnd/NFModelicaBuiltin.mo index 421ca0ebe78..92204d4a2e0 100644 --- a/OMCompiler/Compiler/NFFrontEnd/NFModelicaBuiltin.mo +++ b/OMCompiler/Compiler/NFFrontEnd/NFModelicaBuiltin.mo @@ -436,7 +436,7 @@ function symmetric "Returns a symmetric matrix" end symmetric; function smooth "Indicate smoothness of expression" - parameter input Integer p; + parameter input Integer p annotation(__OpenModelica_functionVariability=true); input RealArrayOrRecord expr; output RealArrayOrRecord s; external "builtin"; @@ -535,8 +535,8 @@ function sample = $overload(OMC_NO_CLOCK.sample, OMC_CLOCK.sample) package OMC_NO_CLOCK impure function sample "Overloaded operator to either trigger time events or to convert between continuous-time and clocked-time representation" - parameter input Real start; - parameter input Real interval; + parameter input Real start annotation(__OpenModelica_functionVariability=true); + parameter input Real interval annotation(__OpenModelica_functionVariability=true); output Boolean b; external "builtin"; annotation(Documentation(info=" @@ -559,8 +559,8 @@ end OMC_CLOCK; function shiftSample<__Any> "First activation of clock is shifted in time" input __Any u; - parameter input Integer shiftCounter(min = 0); - parameter input Integer resolution(min = 1) = 1; + parameter input Integer shiftCounter(min = 0) annotation(__OpenModelica_functionVariability=true); + parameter input Integer resolution(min = 1) = 1 annotation(__OpenModelica_functionVariability=true); output __Any c; external "builtin"; annotation(__OpenModelica_builtin=true, version="Modelica 3.3", Documentation(info=" @@ -570,8 +570,8 @@ end shiftSample; function backSample<__Any> "First activation of clock is shifted in time before activation of u" input __Any u; - parameter input Integer backCounter(min = 0); - parameter input Integer resolution(min = 1) = 1; + parameter input Integer backCounter(min = 0) annotation(__OpenModelica_functionVariability=true); + parameter input Integer resolution(min = 1) = 1 annotation(__OpenModelica_functionVariability=true); output __Any c; external "builtin"; annotation(__OpenModelica_builtin=true, version="Modelica 3.3", Documentation(info=" @@ -729,7 +729,7 @@ encapsulated package Connections function potentialRoot input VariableName node; - parameter input Integer priority = 0; + parameter input Integer priority = 0 annotation(__OpenModelica_functionVariability=true); external "builtin"; annotation(__OpenModelica_builtin=true); end potentialRoot; @@ -859,8 +859,8 @@ function spatialDistribution "Not yet implemented" input Real in1; input Real x; input Boolean positiveVelocity; - parameter input Real initialPoints[:](each min = 0, each max = 1) = {0.0, 1.0}; - parameter input Real initialValues[size(initialPoints, 1)] = {0.0, 0.0}; + parameter input Real initialPoints[:](each min = 0, each max = 1) = {0.0, 1.0} annotation(__OpenModelica_functionVariability=true); + parameter input Real initialValues[size(initialPoints, 1)] = {0.0, 0.0} annotation(__OpenModelica_functionVariability=true); output Real out0; output Real out1; external "builtin"; @@ -1043,7 +1043,7 @@ package Internal "Contains internal implementations, e.g. overloaded builtin fun function rationalClock input Integer intervalCounter(min=0); - parameter input Integer resolution(min = 1) = 1; + parameter input Integer resolution(min = 1) = 1 annotation(__OpenModelica_functionVariability=true); output Clock c; external "builtin"; end rationalClock; @@ -1070,35 +1070,35 @@ package Internal "Contains internal implementations, e.g. overloaded builtin fun impure function subSampleExpression<__Any> input __Any u; - parameter input Integer factor(min=0)=0; + parameter input Integer factor(min=0)=0 annotation(__OpenModelica_functionVariability=true); output __Any y; external "builtin" y=subSample(u,factor); end subSampleExpression; impure function subSampleClock input Clock u; - parameter input Integer factor(min=0)=0; + parameter input Integer factor(min=0)=0 annotation(__OpenModelica_functionVariability=true); output Clock y; external "builtin" y=subSample(u,factor); end subSampleClock; impure function superSampleExpression<__Any> input __Any u; - parameter input Integer factor(min=0)=0; + parameter input Integer factor(min=0)=0 annotation(__OpenModelica_functionVariability=true); output __Any y; external "builtin" y=superSample(u,factor); end superSampleExpression; impure function superSampleClock input Clock u; - parameter input Integer factor(min=0)=0; + parameter input Integer factor(min=0)=0 annotation(__OpenModelica_functionVariability=true); output Clock y; external "builtin" y=superSample(u,factor); end superSampleClock; impure function delay2 input Real expr; - parameter input Real delayTime; + parameter input Real delayTime annotation(__OpenModelica_functionVariability=true); output Real value; algorithm value := delay3(expr, delayTime, delayTime); @@ -1107,7 +1107,7 @@ package Internal "Contains internal implementations, e.g. overloaded builtin fun impure function delay3 input Real expr, delayTime; - parameter input Real delayMax; + parameter input Real delayMax annotation(__OpenModelica_functionVariability=true); output Real value; external "builtin" value=delay(expr, delayTime, delayMax); end delay3; diff --git a/testsuite/flattening/modelica/scodeinst/FuncVariability.mo b/testsuite/flattening/modelica/scodeinst/FuncVariability.mo deleted file mode 100644 index d642d16e606..00000000000 --- a/testsuite/flattening/modelica/scodeinst/FuncVariability.mo +++ /dev/null @@ -1,26 +0,0 @@ -// name: FuncVariability -// keywords: -// status: incorrect -// cflags: -d=newInst -// -// Checks that variability is checked on function parameters. -// - -function f - parameter input Real x; - output Real y = x; -end f; - -model FuncVariability - Real x = f(time); -end FuncVariability; - -// Result: -// Error processing file: FuncVariability.mo -// [flattening/modelica/scodeinst/FuncVariability.mo:15:3-15:19:writable] Error: Function argument x=time in call to f has variability continuous which is not a parameter expression. -// -// # Error encountered! Exiting... -// # Please check the error message and the flags. -// -// Execution failed! -// endResult diff --git a/testsuite/flattening/modelica/scodeinst/FunctionParamVariability.mo b/testsuite/flattening/modelica/scodeinst/FunctionParamVariability.mo new file mode 100644 index 00000000000..4af90a78da4 --- /dev/null +++ b/testsuite/flattening/modelica/scodeinst/FunctionParamVariability.mo @@ -0,0 +1,29 @@ +// name: FunctionParamVariability +// keywords: +// status: correct +// cflags: -d=newInst +// +// Checks that declaring a function parameter constant/parameter has no impact +// on the function, since variability prefixes have no semantic meaning for +// function parameters. +// + +function f + constant input Real x = 1.0; + parameter output Real y = x + x; +end f; + +model FunctionParamVariability + Real x = f(time); +end FunctionParamVariability; + +// Result: +// function f +// input Real x = 1.0; +// output Real y = x + x; +// end f; +// +// class FunctionParamVariability +// Real x = f(time); +// end FunctionParamVariability; +// endResult diff --git a/testsuite/flattening/modelica/scodeinst/Makefile b/testsuite/flattening/modelica/scodeinst/Makefile index d904f5e30d7..5604688b962 100644 --- a/testsuite/flattening/modelica/scodeinst/Makefile +++ b/testsuite/flattening/modelica/scodeinst/Makefile @@ -624,7 +624,6 @@ FuncStringInvalid2.mo \ FuncUnknownDim1.mo \ FuncUnknownDim2.mo \ FuncUnknownDim3.mo \ -FuncVariability.mo \ FuncViaComp.mo \ FuncViaComp2.mo \ FuncViaComp3.mo \ @@ -667,6 +666,7 @@ FunctionMultiOutput4.mo \ FunctionMultiOutput5.mo \ FunctionNoOutput1.mo \ FunctionNonInputOutputParameter.mo \ +FunctionParamVariability.mo \ FunctionPartialDerivative1.mo \ FunctionPartialDerivative2.mo \ FunctionPartialDerivative3.mo \ diff --git a/testsuite/flattening/modelica/scodeinst/Ticket5821.mo b/testsuite/flattening/modelica/scodeinst/Ticket5821.mo index 6fbce9a897e..fc23e2a98ae 100644 --- a/testsuite/flattening/modelica/scodeinst/Ticket5821.mo +++ b/testsuite/flattening/modelica/scodeinst/Ticket5821.mo @@ -2169,8 +2169,8 @@ end Test_total; // input Modelica.Blocks.Types.ExternalCombiTimeTable tableID; // input Integer icol; // input Real timeIn; -// discrete input Real nextTimeEvent; -// discrete input Real pre_nextTimeEvent; +// input Real nextTimeEvent; +// input Real pre_nextTimeEvent; // output Real y; // // external "C" y = ModelicaStandardTables_CombiTimeTable_getValue(tableID, icol, timeIn, nextTimeEvent, pre_nextTimeEvent);