Add newtonian line search#101
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a “direct Newton + line search” pathway for constrained Newton steps to avoid the precision loss from normal equations, and wires it into the equilibrium/thermo solvers (with regression tests for ill-conditioned cases).
Changes:
- Add
constrained_newton_step/constrained_newton_trialhelpers to solve square Newton systems directly with a KKT fallback only for active bounds. - Update
equilibrate_tp,equilibrate_uv, andphase_equilibrate_tpto use the new step + backtracking acceptance based on reduced nonlinear residual. - Add C++ and Python regression tests targeting ill-conditioned equilibrium/Newton scenarios; apply broad C++ reference/spacing formatting updates and tweak pre-commit hooks.
Reviewed changes
Copilot reviewed 8 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_thermo.cpp | Adds unit tests for the new constrained Newton utilities. |
| tests/test_equilibrium.py | Adds a Python regression test for an ill-conditioned multiphase equilibrium Newton step. |
| tests/test_equilibrium.cpp | Minor formatting adjustment in test instantiation lambda signature. |
| src/utils/molar_mass.hpp | Formatting-only: reference spacing normalization. |
| src/utils/molar_mass.cpp | Formatting-only: reference spacing normalization. |
| src/thermo/equilibrate_uv.h | Switches KKT solve to constrained_newton_step and adds backtracking acceptance over coupled UV residual. |
| src/thermo/equilibrate_tp.h | Switches KKT solve to constrained_newton_step and adds residual-reducing backtracking line search. |
| src/species.hpp | Formatting-only: reference spacing normalization. |
| src/species.cpp | Formatting-only: reference spacing normalization. |
| src/math/constrained_newton.h | New header implementing direct Newton solve with KKT fallback and bounded trial-state formation. |
| src/equilibrium/phase_equilibrate_tp.h | Uses new constrained Newton step + line search; improves convergence target handling via log1p/epsilon floor. |
| src/equilibrium/equilibrium.hpp | Formatting-only: reference spacing normalization. |
| src/equilibrium/equilibrium.cpp | Formatting-only: reference spacing normalization. |
| src/equilibrium/equilibrium_options.cpp | Formatting-only: reference spacing normalization. |
| src/equilibrium/equilibrium_dispatch.hpp | Formatting-only: reference spacing normalization. |
| src/equilibrium/equilibrium_dispatch.cpp | Formatting-only: reference spacing normalization. |
| python/csrc/pyequilibrium.cpp | Formatting-only: reference spacing normalization. |
| python/csrc/kintera.cpp | Formatting-only: reference spacing normalization. |
| .pre-commit-config.yaml | Switches the clang-format hook source repo/revision. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+283
to
288
| for (int i = 0; i < nspecies; ++i) { | ||
| if (stoich[i * nreaction + j] < 0 && xfrac0[i] > 0.) { | ||
| log_frac_sum += | ||
| (-stoich[i * nreaction + j]) * log(xfrac0[i] / trial_xg); | ||
| } | ||
| } |
Comment on lines
+39
to
+52
| TEST(LeastSquaresKkt, LineSearchCanScaleNewtonDirectionToBound) { | ||
| double matrix[] = {1.}; | ||
| double constraint[] = {1.}; | ||
| double bound[] = {1.}; | ||
| double rhs[] = {2.}; | ||
| int max_iter = 10; | ||
|
|
||
| int status = | ||
| constrained_newton_step(rhs, matrix, constraint, bound, 1, 1, &max_iter); | ||
|
|
||
| EXPECT_EQ(status, 0); | ||
| EXPECT_DOUBLE_EQ(rhs[0], 2.); | ||
| EXPECT_EQ(max_iter, 1); | ||
| } |
Comment on lines
+21
to
+24
| * The direct solve avoids the loss of precision caused by forming A^T.A. A | ||
| * direction that violates a bound at unit length is still usable when a | ||
| * positive scaling makes it feasible. Only a direction pointing out of an | ||
| * already-active bound is sent through constrained least squares. |
Comment on lines
+303
to
306
| if (!accepted) { | ||
| kkt_err = 3; | ||
| break; | ||
| } |
Comment on lines
+320
to
323
| if (!accepted) { | ||
| err_code = 4; | ||
| break; | ||
| } |
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
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.
No description provided.