Skip to content

Commit

Permalink
Refactor symbolic expression / formula (RobotLocomotion#14020)
Browse files Browse the repository at this point in the history
* Refactor symbolic expression / formula

 - Adds [[nodiscard]] attributes to member functions in order to highlight at
   compile time which return values should not be ignored. Update the affected
   test cases.
 - Make deleted methods public so that a compiler can give a proper
   warning (delete method). Otherwise, a compiler indicates that the method is
   private.
 - modernize-pass-by-value. Read our C++ guideline, https://drake.mit.edu/styleguide/cppguide.html#Pass_By_Value
 - `auto&` => `const auto&` in for-loop
 - Remove redundant declarations in symbolic_{expression,formula}_cell.h as they
   are already in symbolic_{expression,formula}.h
  • Loading branch information
soonho-tri authored Sep 6, 2020
1 parent ced22cb commit 9c30d5b
Show file tree
Hide file tree
Showing 10 changed files with 449 additions and 775 deletions.
9 changes: 5 additions & 4 deletions common/symbolic_expression.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ double Expression::Evaluate(const Environment& env,
if (random_generator == nullptr) {
return ptr_->Evaluate(env);
} else {
Environment env_with_random_variables{env};
return ptr_->Evaluate(
PopulateRandomVariables(env, GetVariables(), random_generator));
}
Expand Down Expand Up @@ -361,7 +360,8 @@ Expression& Expression::operator--() {
}

Expression Expression::operator--(int) {
const Expression copy(*this);
// Declare as non-const to allow move.
Expression copy(*this);
--*this;
return copy;
}
Expand Down Expand Up @@ -557,6 +557,7 @@ class PrecisionGuard {
: os_{os}, original_precision_{os->precision()} {
os_->precision(new_precision);
}
DRAKE_NO_COPY_NO_MOVE_NO_ASSIGN(PrecisionGuard)
~PrecisionGuard() { os_->precision(original_precision_); }

private:
Expand Down Expand Up @@ -938,12 +939,12 @@ class IsAffineVisitor {
found_non_affine_element_ = found_non_affine_element_ || IsNotAffine(e);
}

bool result() const { return !found_non_affine_element_; }
[[nodiscard]] bool result() const { return !found_non_affine_element_; }

private:
// Returns true if `e` is *not* affine in variables_ (if exists) or all
// variables in `e`.
bool IsNotAffine(const Expression& e) const {
[[nodiscard]] bool IsNotAffine(const Expression& e) const {
if (!e.is_polynomial()) {
return true;
}
Expand Down
58 changes: 33 additions & 25 deletions common/symbolic_expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ symbolic::Expression can be used as a scalar type of Eigen types.
class Expression {
public:
DRAKE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN(Expression)
~Expression() = default;

/** Default constructor. It constructs Zero(). */
Expression() { *this = Zero(); }
Expand All @@ -208,9 +209,9 @@ class Expression {
// NOLINTNEXTLINE(runtime/explicit): This conversion is desirable.
Expression(const Variable& var);
/** Returns expression kind. */
ExpressionKind get_kind() const;
[[nodiscard]] ExpressionKind get_kind() const;
/** Collects variables in expression. */
Variables GetVariables() const;
[[nodiscard]] Variables GetVariables() const;

/** Checks structural equality.
*
Expand Down Expand Up @@ -239,15 +240,15 @@ class Expression {
*
* (p1.Expand() - p2.Expand()).EqualTo(0).
*/
bool EqualTo(const Expression& e) const;
[[nodiscard]] bool EqualTo(const Expression& e) const;

/** Provides lexicographical ordering between expressions.
This function is used as a compare function in map<Expression> and
set<Expression> via std::less<drake::symbolic::Expression>. */
bool Less(const Expression& e) const;
[[nodiscard]] bool Less(const Expression& e) const;

/** Checks if this symbolic expression is convertible to Polynomial. */
bool is_polynomial() const;
[[nodiscard]] bool is_polynomial() const;

/** Evaluates using a given environment (by default, an empty environment) and
* a random number generator. If there is a random variable in this expression
Expand Down Expand Up @@ -279,7 +280,7 @@ class Expression {
*
* @throws std::runtime_error if NaN is detected during evaluation.
*/
Expression EvaluatePartial(const Environment& env) const;
[[nodiscard]] Expression EvaluatePartial(const Environment& env) const;

/** Returns true if this symbolic expression is already
* expanded. Expression::Expand() uses this flag to avoid calling
Expand All @@ -290,7 +291,7 @@ class Expression {
* that the expression is not expanded. This is because exact checks can be
* costly and we want to avoid the exact check at the construction time.
*/
bool is_expanded() const;
[[nodiscard]] bool is_expanded() const;

/** Expands out products and positive integer powers in expression. For
* example, `(x + 1) * (x - 1)` is expanded to `x^2 - 1` and `(x + y)^2` is
Expand All @@ -301,37 +302,38 @@ class Expression {
*
* @throws std::runtime_error if NaN is detected during expansion.
*/
Expression Expand() const;
[[nodiscard]] Expression Expand() const;

/** Returns a copy of this expression replacing all occurrences of @p var
* with @p e.
* @throws std::runtime_error if NaN is detected during substitution.
*/
Expression Substitute(const Variable& var, const Expression& e) const;
[[nodiscard]] Expression Substitute(const Variable& var,
const Expression& e) const;

/** Returns a copy of this expression replacing all occurrences of the
* variables in @p s with corresponding expressions in @p s. Note that the
* substitutions occur simultaneously. For example, (x / y).Substitute({{x,
* y}, {y, x}}) gets (y / x).
* @throws std::runtime_error if NaN is detected during substitution.
*/
Expression Substitute(const Substitution& s) const;
[[nodiscard]] Expression Substitute(const Substitution& s) const;

/** Differentiates this symbolic expression with respect to the variable @p
* var.
* @throws std::runtime_error if it is not differentiable.
*/
Expression Differentiate(const Variable& x) const;
[[nodiscard]] Expression Differentiate(const Variable& x) const;

/** Let `f` be this Expression, computes a row vector of derivatives,
* `[∂f/∂vars(0), ... , ∂f/∂vars(n-1)]` with respect to the variables
* @p vars.
*/
RowVectorX<Expression> Jacobian(
[[nodiscard]] RowVectorX<Expression> Jacobian(
const Eigen::Ref<const VectorX<Variable>>& vars) const;

/** Returns string representation of Expression. */
std::string to_string() const;
[[nodiscard]] std::string to_string() const;

/** Returns zero. */
static Expression Zero();
Expand Down Expand Up @@ -876,14 +878,14 @@ class uniform_real_distribution<drake::symbolic::Expression> {
}

/// Returns the minimum value a.
RealType a() const { return a_; }
[[nodiscard]] RealType a() const { return a_; }
/// Returns the maximum value b.
RealType b() const { return b_; }
[[nodiscard]] RealType b() const { return b_; }

/// Returns the minimum potentially generated value.
result_type min() const { return a_; }
[[nodiscard]] result_type min() const { return a_; }
/// Returns the maximum potentially generated value.
result_type max() const { return b_; }
[[nodiscard]] result_type max() const { return b_; }

private:
using Variable = drake::symbolic::Variable;
Expand Down Expand Up @@ -1009,18 +1011,22 @@ class normal_distribution<drake::symbolic::Expression> {
}

/// Returns the mean μ distribution parameter.
RealType mean() const { return mean_; }
[[nodiscard]] RealType mean() const { return mean_; }
/// Returns the deviation σ distribution parameter.
RealType stddev() const { return stddev_; }
[[nodiscard]] RealType stddev() const { return stddev_; }

/// Returns the minimum potentially generated value.
///
/// @note In libstdc++ std::normal_distribution<> defines min() and max() to
/// return -DBL_MAX and DBL_MAX while the one in libc++ returns -INFINITY and
/// INFINITY. We follows libc++ and return -INFINITY and INFINITY.
result_type min() const { return -std::numeric_limits<double>::infinity(); }
/// Returns the maximum potentially generated value.
result_type max() const { return std::numeric_limits<double>::infinity(); }
[[nodiscard]] result_type min() const {
return -std::numeric_limits<double>::infinity();
}
/// Returns the maximum potentially generated value.o
[[nodiscard]] result_type max() const {
return std::numeric_limits<double>::infinity();
}

private:
using Variable = drake::symbolic::Variable;
Expand Down Expand Up @@ -1110,15 +1116,17 @@ class exponential_distribution<drake::symbolic::Expression> {
}

/// Returns the lambda λ distribution parameter.
RealType lambda() const { return lambda_; }
[[nodiscard]] RealType lambda() const { return lambda_; }
/// Returns the minimum potentially generated value.
result_type min() const { return 0.0; }
[[nodiscard]] result_type min() const { return 0.0; }

/// Returns the maximum potentially generated value.
/// @note that in libstdc++ exponential_distribution<>::max() returns DBL_MAX
/// while the one in libc++ returns INFINITY. We follows libc++ and return
/// INFINITY.
result_type max() const { return std::numeric_limits<double>::infinity(); }
[[nodiscard]] result_type max() const {
return std::numeric_limits<double>::infinity();
}

private:
using Variable = drake::symbolic::Variable;
Expand Down
Loading

0 comments on commit 9c30d5b

Please sign in to comment.