-
Notifications
You must be signed in to change notification settings - Fork 205
Draft expression templates for functions #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
11c2544 to
0553570
Compare
To add back a bit of flexibility solvers consume arbitrary functions and operators +,-,* on top of functions are supported.
0553570 to
e45e101
Compare
| #define INCLUDE_CPPOPTLIB_FUNCTION_H_ | ||
|
|
||
| #include "Eigen/Core" | ||
| #include "function_base.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Include the directory when naming header files [build/include_subdir] [4]
|
|
||
| #include "Eigen/Core" | ||
| #include "function_base.h" | ||
| #include "function_expressions.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Include the directory when naming header files [build/include_subdir] [4]
| #include "Eigen/Core" | ||
| #include "function_base.h" | ||
| #include "function_expressions.h" | ||
| #include "function_penalty.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Include the directory when naming header files [build/include_subdir] [4]
| #include "function_base.h" | ||
| #include "function_expressions.h" | ||
| #include "function_penalty.h" | ||
| #include "function_problem.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Include the directory when naming header files [build/include_subdir] [4]
| } | ||
| } | ||
|
|
||
| virtual std::unique_ptr<FunctionInterface<TScalar, TMode, TDimension>> clone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
"virtual" is redundant since function is already declared as "override" [readability/inheritance] [4]
| // Shrink the simplex toward the best vertex. | ||
| void shrink(matrix_t &s, std::vector<int> &idx, std::vector<scalar_t> &f, | ||
| const function_t &function) { | ||
| void shrink(MatrixType &s, std::vector<int> &idx, std::vector<ScalarType> &f, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Is this a non-const reference? If so, make const or use a pointer: std::vector &f [runtime/references] [2]
| #include <utility> | ||
|
|
||
| #include "../function.h" | ||
| #include "progress.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Include the directory when naming header files [build/include_subdir] [4]
|
|
||
| #include "cppoptlib/function.h" | ||
|
|
||
| using namespace cppoptlib::function; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Do not use namespace using-directives. Use using-declarations instead. [build/namespaces] [5]
| : public cppoptlib::function::FunctionCRTP<ConstantFunction, double, | ||
| DifferentiabilityMode::None> { | ||
| double c; | ||
| ConstantFunction(double c_) : c(c_) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Single-parameter constructors should be marked explicit. [runtime/explicit] [4]
| MatrixType *hessian = nullptr | ||
|
|
||
| ) const override { | ||
| ) const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[cpplint] reported by reviewdog 🐶
Closing ) should be moved to the previous line [whitespace/parens] [2]
It would be nice to build a type system on top of FunctionXd which supports hessian and gradient computation.
So far the API is
However, with the expressions not being derived from AnyFunctino (which is abstract), template type deduction failes here
as the latter is not an AnyFunction but a MulExpression. The big advantage is that we can write