Solver: Iterative Hessian Sketch #119
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
vp314
left a comment
vp314
left a comment
There was a problem hiding this comment.
Minor changes requested. One small addition to the code base to check that the rows of the coefficient matrix exceed the columns. Another test change addition to see if everything works when the dimension of the compression is larger than the number of rows of the coefficient matrix.
| @misc{pilanci2014iterative, | ||
| title = {Iterative {{Hessian}} Sketch: {{Fast}} and Accurate Solution Approximation for Constrained Least-Squares}, | ||
| shorttitle = {Iterative {{Hessian}} Sketch}, | ||
| author = {Pilanci, Mert and Wainwright, Martin J.}, | ||
| year = {2014}, | ||
| publisher = {arXiv}, | ||
| doi = {10.48550/ARXIV.1411.0347}, | ||
| } |
There was a problem hiding this comment.
I think this reference is listed twice. I think the JMLR version (linked in the PR description) should be the appropriate reference to use:
@article{pilanci2016iterative,
title={Iterative Hessian sketch: Fast and accurate solution approximation for constrained least-squares},
author={Pilanci, Mert and Wainwright, Martin J},
journal={Journal of Machine Learning Research},
volume={17},
number={53},
pages={1--38},
year={2016}
}
| # Mathematical Description | ||
| Let ``A \\in \\mathbb{R}^{m \\times n}`` and consider the least square problem ``\\min_x | ||
| \\|Ax - b \\|_2^2``. If we let ``S \\in \\mathbb{R}^{s \\times m}`` be a compression matrix, then |
There was a problem hiding this comment.
Perhaps add a comment m >> n
| Iterative Hessian Sketch iteratively finds a solution to this problem | ||
| by repeatedly updating ``x_{k+1} = x_k + \\alpha u_k``where ``u_k`` is the solution to the | ||
| convex optimization problem, | ||
| ``u_k = \\min_u \\{\\|S_k Au\\|_2^2 - \\langle A, b - Ax_k \\rangle \\}.`` This method |
There was a problem hiding this comment.
Usually, we write u_k \\in \\argmin_u...
| by repeatedly updating ``x_{k+1} = x_k + \\alpha u_k``where ``u_k`` is the solution to the | ||
| convex optimization problem, | ||
| ``u_k = \\min_u \\{\\|S_k Au\\|_2^2 - \\langle A, b - Ax_k \\rangle \\}.`` This method | ||
| has been to shown to converge geometrically at a rate ``\\rho \\in (0, 1/2]``, typically the |
There was a problem hiding this comment.
I think there should be a period before typically instead of a comma. Then typically should be capitalized.
| ## Keywords | ||
| - `compressor::Compressor`, a technique for forming the compressed linear system. | ||
| - `log::Logger`, a technique for logging the progress of the solver. | ||
| - `error::SolverError', a method for estimating the progress of the solver. |
There was a problem hiding this comment.
| - `error::SolverError', a method for estimating the progress of the solver. | |
| - `error::SolverError`, a method for estimating the progress of the solver. |
The single quote should have been whatever this character is: `
| - `log::Logger`, a technique for logging the progress of the solver. | ||
| - `error::SolverError', a method for estimating the progress of the solver. | ||
| - `alpha::Float64`, a step size parameter. | ||
There was a problem hiding this comment.
Throw in a warning or an information admonition with some guidance about the value of alpha
| logger = complete_logger(ingredients.log) | ||
| error = complete_error(ingredients.error, ingredients, A, b) | ||
| sample_size::Int64 = compressor.n_rows | ||
| rows_a, cols_a = size(A) |
There was a problem hiding this comment.
Add check if rows_a <= cols_a and throw an error. Make sure to test for this error
|
|
||
| end | ||
|
|
||
| @testset "IHS: rsolve!" begin |
There was a problem hiding this comment.
We should add an rsolve! test that looks at when com_dim > size(A,1). This is not a sensible thing to do in practice, but we should be sure it is handled.
| solver.R = UpperTriangular(qr!(solver.mat_view).R) | ||
| # Compute first R' solver R'R x = g | ||
| ldiv!(solver.buffer_vec, solver.R', solver.gradient_vec) | ||
| # Compute second R Solve Rx = (R')^(-1)g will be stored in gradient_vec | ||
| ldiv!(solver.gradient_vec, solver.R, solver.buffer_vec) | ||
| # update the solution | ||
| # solver.solution_vec = solver.solution_vec + alpha * solver.gradient_vec |
There was a problem hiding this comment.
We should add "Iterative Hessian Sketch" to the wiki page (either v0.2 or v0.3, your choice). And indicate that we want to allow alternative subsolvers in the inner loop.
| result = rsolve!(solver_rec, x, A, b) | ||
| #test that the error decreases | ||
| @test norm(A * x_st - b) > norm(A * x - b) | ||
| @test solver_rec.log.converged |
There was a problem hiding this comment.
Can we also check the iterate that we converge on so that way it is not on iterate 40?
This pull request implements Iterative Hessian Sketch solver from Pilanci and Wainwright. The commit includes:
complete_solverfunctionrsolve!function