-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add alternative form of weighted Gauss-Newton nonlinear solve #723
Conversation
looks good, overall! |
I've added the non-trivial test, which actually exposed a bug that I had previously hacked around. To resolve that issue I had to add a new nonlinear solver registry since the standard weighted Gauss-Newton registry assumes that |
d11_(&system){ | ||
// resize leading dimension according to weighing operator | ||
pressio::ops::resize(d5_, weigher.leading_dim); | ||
pressio::ops::resize(d6_, weigher.leading_dim, pressio::ops::extent(d6_, 1)); |
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.
please change this to avoid public members, so that the weigher class has a method weigher.leadingDim()
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.
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.
see my comment, if you make that change and all tests pass we are good
Adds a new nonlinear solver tag and accompanying functions (including an overload of
create_gauss_newton_solver
) that just redefines the action of the weighting function in the weighted Gauss-Newton solve, such thatThat is, the implied
W^T * W
should be exactly the same as in theW
in the original weighted Gauss-Newton solve, but the order of operation allows for fewer FLOPs and lower memory usage if the first dimension ofW
is smaller than its second dimension. This is particularly important for gappy POD simulations where the first dimension ofW
would be the number of modes, and the second dimension is the number of residual samples.Adds test using the new
CompactWeightedGaussNewtonNormalEqTag
.