Skip to content
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

Repeat call in line_search() #17

Closed
bcollica opened this issue Jun 8, 2021 · 1 comment
Closed

Repeat call in line_search() #17

bcollica opened this issue Jun 8, 2021 · 1 comment

Comments

@bcollica
Copy link
Collaborator

bcollica commented Jun 8, 2021

I think there is an extra call to compute_r() in the line_search() function on line 137 of admm.py. It seems like build_linear_system() returns a value for r on line 128, and then compute_r() is called again on line 137 using the same unchanged arguments.

This is the call on line 128:

M, r = build_linear_system(gamma, u, C, e, rho, s, t)

This is what build_linear_system() calls:

def build_linear_system(gamma, u, C, e, rho, s, t):
    """Build the linear system for ADMM.primal_dual (see appendix section 5.2.7)."""
    n, k = e.shape
    H = hessian_f0(gamma, e, rho, s)
    uC = scipy.sparse.diags(np.squeeze(u), 0).dot(C)
    Cg = scipy.sparse.diags(np.squeeze(C.dot(gamma)))
    M = scipy.sparse.vstack((scipy.sparse.hstack((H, C.T)),
                                               scipy.sparse.hstack((-uC, -Cg)))).tocsr()
    r = compute_r(gamma, u, C, e, rho, s, t)
    return M, r

And then compute_r() called again with the same variables on line 137:

r = compute_r(gamma, u, C, e, rho, s, t)

I'm not sure if this was intentional, nor do I know how computationally taxing it is either since I haven't had a chance to explicitly profile compute_r(). But if it's a redundant call, then it is potentially being called up to 1,500 times more than it should per topic (15 iterations of update_xis inside admm() and then up to 100 iterations of line_search() inside primal_dual()).

@z-chen
Copy link
Collaborator

z-chen commented Nov 4, 2021

Fixed by #23

@z-chen z-chen closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants