Repository navigation
Conversation
…epts is_step_valid() runs on every trial point, which makes it the wrong place for checks that are expensive, or that can only veto a step without the energy reflecting why. Add is_step_acceptable(): it is called only once a trial step has passed the descent criteria, and returning false makes the line search keep backtracking. A problem may change its own discretization before rejecting (e.g. refine a quadrature where it found the step invalid, so the energy there is no longer finite). The line search therefore re-evaluates the energy and gradient at x0 after a rejection, so its criteria keep comparing like with like. If the old direction no longer descends on the changed problem, the line search fails through its usual path so the solver picks a new direction; the "energy at x unchanged" assert in that path is relaxed accordingly. Only Backtracking implements the loop, so Armijo, RobustArmijo and ResidualBacktracking inherit it. NoLineSearch never backtracks and does not call the hook. The default returns true, so existing problems are unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A problem that certifies the whole segment x + t*dx (e.g. polyfem's continuous inversion check) can only trust the configuration the solver takes if that configuration is within a known distance of the segment. - step_along(x, s, dx) forms x + s*dx with std::fma, so each coefficient is rounded once. Every trial and accepted step in the line searches and the final solver update goes through it. - product_rounded_down(a, b) replaces the fesetround(FE_DOWNWARD) block that scaled the step by the max step size. That block needed FENV_ACCESS or -frounding-math to be honored; the FMA-based version is correct in any rounding mode and under any optimization. - Tests: step_along matches a single rounding where a separate multiply and add differ; product_rounded_down never exceeds the exact product. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A rejection may change the problem's discretization (polyfem refines a quadrature where the step was found invalid), and the search direction was computed for the old one. Rather than re-evaluating the energy and gradient at x and backtracking further along that direction, fail the line search through the caller's usual path, so the solver computes a new direction on the problem as it is now. Each line search thus works on a single, fixed problem, and the re-evaluation and descent re-check go away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
+ Coverage 79.45% 79.53% +0.07%
==========================================
Files 55 55
Lines 2940 2951 +11
Branches 377 378 +1
==========================================
+ Hits 2336 2347 +11
Misses 604 604
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a hook for problems that certify steps after the line search accepts them, and makes the steps the solver takes exactly reproducible so a problem can certify them. Needed by polyfem/polyfem#549 (conservative Jacobian checks).
What changes
Problem::is_step_acceptable(x0, x1): new virtual, defaulttrue. The line search calls it on a step that already passed its descent criteria. A rejection ends the line search as failed, so the solver computes a new direction; the problem may change in between (polyfem refines its quadrature there), and each line search stays on one fixed problem.step_along(x, s, dx)forms every trial and accepted step asfma(s, dx, x), one rounding per coefficient, in the line searches and in the solver's final update.product_rounded_down(a, b)replaces thefesetround(FE_DOWNWARD)block that scaled the step bymax_step_size. That block neededFENV_ACCESSor-frounding-mathto be honored; the FMA version is correct in any rounding mode and at any optimization level.Impact on existing users
is_step_acceptabledefaults totrue, so problems that don't override it see no change from the hook.step_alongchanges every solve:x + s*dxwas two roundings per coefficient, now one. Iterates differ from before in the last bit, so results that depend on bitwise reproducibility, or tests with very tight tolerances, can move. polyfem's full test suite passes with it (280/280).std::fmais a hardware instruction on arm64 and on x86-64 with FMA; elsewhere it is a correctly rounded library call, which is slower than a separate multiply and add.Tests
line-search-step-acceptable: the hook is called once per accepted step, a rejection ends the line search (NaN step), and the next line search sees the changed problem.step_alongmatches a single rounding where a separate multiply and add differ;product_rounded_downnever exceeds the exact product.unit_testson this branch: 27 test cases pass (macOS, Apple clang 17, Release).