Conversation
update 11/8
update 5/30
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
==========================================
- Coverage 81.67% 81.46% -0.21%
==========================================
Files 49 49
Lines 2025 2083 +58
Branches 269 274 +5
==========================================
+ Hits 1654 1697 +43
- Misses 371 386 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| virtual double compute_grad_norm(const Problem &objFunc, const TVector &x, const TVector &grad) const | ||
| { | ||
| return objFunc.grad_norm(grad, norm_type_); |
There was a problem hiding this comment.
I believe so. Is there an issue I am overlooking?
| default_init_step_size = params["line_search"]["default_init_step_size"]; | ||
| step_ratio = params["line_search"]["step_ratio"]; | ||
|
|
||
| try_interpolating_step = params["line_search"]["try_interpolating_step"]; |
| virtual double step_norm_rescaling(const std::string &norm_type) const {return 1;} | ||
| virtual double energy_norm_rescaling(const std::string &norm_type) const {return 1;} | ||
|
|
||
| virtual double grad_norm(const TVector &x, const std::string &norm_type) const {return x.norm();} |
There was a problem hiding this comment.
this is super misleading. x is the solution, now it is the grad. which one is it?
| /// @return True if the solver should stop, false otherwise. | ||
| virtual bool stop(const TVector &x) { return false; } | ||
|
|
||
| virtual double grad_norm_rescaling(const std::string &norm_type) const {return 1;} |
There was a problem hiding this comment.
fixed. I use an enum now. It feels a bit awkward as it seems like something that would be more natural to define in the code defining the problem completely (e.g., PolyFEM), but I don't see a better solution at the moment.
| double grad_norm_rescaling = 1; | ||
| bool try_interpolating_step; | ||
| double rel_interpolation_accuracy_tol = 0; | ||
| std::string norm_type = "Euclidean"; |
There was a problem hiding this comment.
Solver::set_line_search
|
|
||
| m_descent_strategy = 0; | ||
|
|
||
| norm_type_ = solver_params["norm_type"]; |
| { | ||
| m_logger.debug("[{}] large (or nan) linear solve residual {}>{} (‖∇f‖={})", | ||
| name(), residual, residual_tolerance * characteristic_length, grad.norm()); | ||
| name(), residual, current_residual_tolerance, objFunc.grad_norm(grad, "L2")); |
| virtual double step_norm_rescaling(const std::string &norm_type) const {return 1;} | ||
| virtual double energy_norm_rescaling(const std::string &norm_type) const {return 1;} | ||
|
|
||
| virtual double grad_norm(const TVector &x, const std::string &norm_type) const {return x.norm();} |
There was a problem hiding this comment.
also why is the problem resposible for this?
update 3/13
maxpaik16
force-pushed
the
rescaled-tolerances
branch
from
cbedb97 to
f842fd4
Compare
| // Solve the linear system Ax = b | ||
| virtual void solve(const Ref<const VectorXd> b, Ref<VectorXd> x) override; | ||
|
|
||
| virtual void set_tolerance(const double tol) override {residual_error_ = tol;} |
| enum class NormType | ||
| { | ||
| EUCLIDEAN = 0, | ||
| L2 = 1, |
There was a problem hiding this comment.
maybe but is this energy norm, right?
|
|
||
| try | ||
| { | ||
| THessian hessian; |
There was a problem hiding this comment.
This is really expensive, we compute the Hessian twice every time?
There was a problem hiding this comment.
and what if there is not hessian?
teseoch
left a comment
teseoch
left a comment
There was a problem hiding this comment.
There is a comment left. still not happy with the norm computation in the problem
2 participants
Added functions to nonlinear Problem class to allow user-specified norms with appropriate rescaling functions
Updated nonlinear solver and line search logic to use these norms and custom rescalings