Merge pull request #3893 from opensim-org/expression_based_parameter_…

…goal_tweaks

A few tweaks to `MocoExpressionBasedParameterGoal`

OpenSim/Common/ExpressionBasedFunction.cpp

@@ -69,11 +69,15 @@ class SimTKExpressionBasedFunction : public SimTK::Function {
             }
         } catch (Lepton::Exception& ex) {
             std::string msg = ex.what();
-            std::string undefinedVar = msg.substr(32, msg.size() - 32);
-            OPENSIM_THROW(Exception, 
-                    fmt::format("Variable '{}' is not defined. Use "
-                    "setVariables() to explicitly define this variable. Or, "
-                    "remove it from the expression.", undefinedVar));
+            if (msg.compare(0, 30, "No value specified for variable")) { 
+                std::string undefinedVar = msg.substr(32, msg.size() - 32);
+                OPENSIM_THROW(Exception, 
+                        fmt::format("Variable '{}' is not defined. Use "
+                        "setVariables() to explicitly define this variable. "
+                        "Or, remove it from the expression.", undefinedVar));
+            } else {
+                OPENSIM_THROW(Exception, "Lepton parsing error: {}", msg);
+            }
         }
     }
 

OpenSim/Moco/MocoGoal/MocoExpressionBasedParameterGoal.cpp

@@ -28,12 +28,12 @@ using namespace OpenSim;
 void MocoExpressionBasedParameterGoal::constructProperties() {
     constructProperty_expression("");
     constructProperty_parameters();
-    constructProperty_variable_names();
+    constructProperty_variables();
 }
 
 void MocoExpressionBasedParameterGoal::initializeOnModelImpl(const Model& model)
         const {
-    OPENSIM_THROW_IF_FRMOBJ(get_expression() == "", Exception,
+    OPENSIM_THROW_IF_FRMOBJ(get_expression().empty(), Exception,
             "The expression has not been set. Use setExpression().")
     m_program = Lepton::Parser::parse(get_expression()).optimize()
                 .createProgram();
@@ -67,24 +67,23 @@ void MocoExpressionBasedParameterGoal::initializeOnModelImpl(const Model& model)
     }
 
     // test to make sure all variables are there
-    try
-    {
+    try {
         std::map<std::string, double> parameterVars;
-        for (int i = 0; i < getProperty_variable_names().size(); ++i) {
-            parameterVars[get_variable_names(i)] = getPropertyValue(i);
+        for (int i = 0; i < getProperty_variables().size(); ++i) {
+            parameterVars[get_variables(i)] = getPropertyValue(i);
         }
         m_program.evaluate(parameterVars);
-    }
-    catch (Lepton::Exception& ex)
-    {
-        std::string msg = ex.what();
-        std::string help = "";
+    } catch (Lepton::Exception& ex) {
+        const std::string msg = ex.what();
         if (msg.compare(0, 30, "No value specified for variable")) {
-            help = " Use addParameter() to add a parameter for this variable, "
-                   "or remove the variable from the expression for this goal.";
+            std::string undefinedVar = msg.substr(32, msg.size() - 32);
+            OPENSIM_THROW_FRMOBJ(Exception, 
+                    fmt::format("Parameter variable '{}' is not defined. Use "
+                    "addParameter() to explicitly define this variable. Or, "
+                    "remove it from the expression.", undefinedVar));
+        } else {
+            OPENSIM_THROW_FRMOBJ(Exception, "Lepton parsing error: {}", msg);
         }
-        OPENSIM_THROW_FRMOBJ(Exception, fmt::format("Expression evaluate error:"
-                                                    " {}.{}", msg, help));
     }
 }
 
@@ -110,16 +109,16 @@ double MocoExpressionBasedParameterGoal::getPropertyValue(int i) const {
 void MocoExpressionBasedParameterGoal::calcGoalImpl(
         const GoalInput& input, SimTK::Vector& values) const {
     std::map<std::string, double> parameterVars;
-    for (int i = 0; i < getProperty_variable_names().size(); ++i) {
-        parameterVars[get_variable_names(i)] = getPropertyValue(i);
+    for (int i = 0; i < getProperty_variables().size(); ++i) {
+        parameterVars[get_variables(i)] = getPropertyValue(i);
     }
     values[0] = m_program.evaluate(parameterVars);
 }
 
 void MocoExpressionBasedParameterGoal::printDescriptionImpl() const {
     log_info("        expression: {}", get_expression());
     for (int i = 0; i < getProperty_parameters().size(); ++i) {
-        log_info("        variable {}: {}", get_variable_names(i),
+        log_cout("        variable {}: {}", get_variables(i),
                  get_parameters(i).getName());
     }
 }

OpenSim/Moco/MocoGoal/MocoExpressionBasedParameterGoal.h

@@ -26,18 +26,19 @@
 namespace OpenSim {
 class Model;
 
-/** Minimize an arithmetic expression of parameters. This goal supports any
-number of MocoParameters that are combined into a single goal. The expression
-string should match the Lepton (lightweight expression parser) format.
+/** Minimize or constrain an arithmetic expression of parameters. 
+
+This goal supports both "cost" and "endpoint constraint" modes and can be 
+defined using any number of MocoParameters. The expression string should match 
+the Lepton (lightweight expression parser) format.
 
 # Creating Expressions
 
 Expressions can be any string that represents a mathematical expression, e.g.,
 "x*sqrt(y-8)". Expressions can contain variables, constants, operations,
-parentheses, commas, spaces, and scientific e notation. The full list of
-operations (also in Lepton::Operation) is:
-sqrt, exp, log, sin, cos, sec, csc, tan, cot, asin, acos, atan, sinh, cosh,
-tanh, erf, erfc, step, delta, square, cube, recip, min, max, abs, as well as
+parentheses, commas, spaces, and scientific "e" notation. The full list of
+operations is: sqrt, exp, log, sin, cos, sec, csc, tan, cot, asin, acos, atan, 
+sinh, cosh, tanh, erf, erfc, step, delta, square, cube, recip, min, max, abs, 
 +, -, *, /, and ^.
 
 # Examples
@@ -50,7 +51,7 @@ auto* spring2_parameter = mp.addParameter("spring2_stiffness", "spring2",
 auto* spring_goal = mp.addGoal<MocoExpressionBasedParameterGoal>();
 double STIFFNESS = 100.0;
 // minimum is when p + q = STIFFNESS
-spring_goal->setExpression(fmt::format("square( p+q-{} )", STIFFNESS));
+spring_goal->setExpression(fmt::format("square(p+q-{})", STIFFNESS));
 spring_goal->addParameter(*spring1_parameter, "p");
 spring_goal->addParameter(*spring2_parameter, "q");
 @endcode
@@ -75,10 +76,10 @@ class OSIMMOCO_API MocoExpressionBasedParameterGoal : public MocoGoal {
         set_expression(std::move(expression));
     }
 
-    /** Set the mathematical expression to minimize. Variable names should match
-    the names set with addParameter(). See "Creating Expressions" in the class
-    documentation above for an explanation of how to create expressions.
-    */
+    /** Set the arithmetic expression to minimize or constrain. Variable 
+    names should match the names set with addParameter(). See "Creating 
+    Expressions" in the class documentation above for an explanation of how to 
+    create expressions. */
     void setExpression(std::string expression) {
         set_expression(std::move(expression));
     }
@@ -87,9 +88,9 @@ class OSIMMOCO_API MocoExpressionBasedParameterGoal : public MocoGoal {
     expression string. All variables in the expression must have a corresponding
     parameter, but parameters with variables that are not in the expression are
     ignored. */
-    void addParameter(const MocoParameter& parameter, std::string variableName) {
+    void addParameter(const MocoParameter& parameter, std::string variable) {
         append_parameters(parameter);
-        append_variable_names(std::move(variableName));
+        append_variables(std::move(variable));
     }
 
 protected:
@@ -104,15 +105,15 @@ class OSIMMOCO_API MocoExpressionBasedParameterGoal : public MocoGoal {
 
     /** Get the value of the property from its index in the property_refs vector.
     This will use m_data_types to get the type, and if it is a Vec type, it uses
-    m_indices to get the element to return, both at the same index i.*/
+    m_indices to get the element to return, both at the same index i. */
     double getPropertyValue(int i) const;
 
     OpenSim_DECLARE_PROPERTY(expression, std::string,
-            "The expression string with variables q0-q9.");
+            "The expression string defining this cost or endpoint constraint.");
     OpenSim_DECLARE_LIST_PROPERTY(parameters, MocoParameter,
-            "MocoParameters to use in the expression.");
-    OpenSim_DECLARE_LIST_PROPERTY(variable_names, std::string,
-            "Variable names of the MocoParameters to use in the expression.");
+            "Parameters included in the expression.");
+    OpenSim_DECLARE_LIST_PROPERTY(variables, std::string,
+            "Variables names corresponding to parameters in the expression.");
 
     mutable Lepton::ExpressionProgram m_program;
     // stores references to one property per parameter