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

solve_bicgstab: avoid iter==1 check #3618

Conversation

eebasso
Copy link
Contributor

@eebasso eebasso commented Nov 8, 2023

Summary

This PR restructures solve_bicgstab such that the iter==1 check in the iter loop is avoided.

Additional background

This is one of the split off changes to the CG and BiCGSTAB implementations in MLCGSolver.H

@WeiqunZhang
Copy link
Member

Could you please explain why the change is made? To prepare for more changes or just to avoid the iter == 1 test? But now rho = dotxy(rh,r); appears twice.

@eebasso
Copy link
Contributor Author

eebasso commented Nov 8, 2023

The main reason is to avoid the iter==1 test, but it will also help prepare for more changes related to the larger PR suggesting improvements to the BiCGSTAB implementation. It's true that now rho = dotxy(rh,r) appears twice, but I currently can't think of a way to eliminate that while also avoiding the iter==1 test. One way to look at it is that everything above the iter loop is essentially the iter=0 initialization of the variables, including sol, r, rh, p, and rho.

@WeiqunZhang
Copy link
Member

I think if (iter == 1) is much less an evil than duplicating rho = dotxy(rh,r);. Even if the latter were slightly better, that would not be a good reason to make the change, because any changes have the potential of introducing bugs.

@eebasso
Copy link
Contributor Author

eebasso commented Nov 13, 2023

Okay, it was been rewritten so that rho = dotxy(rh,r) is not duplicated. The unnecessary initializations of alpha, omega, and rho_1 have also been eliminated. As far as bugs are concerned, what do the tests show?

@eebasso eebasso closed this Nov 17, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants