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

Add support for an alternative inverter stopping criterion th more suitable for heavy quarks. #80

Closed
jpfoley opened this issue Sep 4, 2012 · 9 comments
Assignees

Comments

@jpfoley
Copy link
Member

jpfoley commented Sep 4, 2012

This was already mentioned as part of another issue, which has since been closed. Basically, most of the necessary code modifications have already been implemented in a side branch. That code requires a just a few tweaks before it can be merged with the master branch. This is something MILC has been looking for for a while, and there doesn't seem to be any reason not to include it in the next release.

@maddyscientist
Copy link
Member

The only question is how we select this. Will this be another option we to QudaInvertParam?

I have been mucking around with the solvers, so please hold off doing merging this until I have merged (today or tomorrow).


This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by

reply email and destroy all copies of the original message.

@ghost ghost assigned jpfoley and maddyscientist Sep 4, 2012
@jpfoley
Copy link
Member Author

jpfoley commented Sep 4, 2012

QudaInvertParam seems to be the path of least resistance. Unfortunately, it's already pretty crowded. I guess eventually you'll want to try to factorise the data in there. I won't touch anything until you've merged your changes. I'm always happy not to do stuff :P.

@maddyscientist
Copy link
Member

For what solvers do you need this? For all of CG, multi-shift CG and BiCGstab?

@jpfoley
Copy link
Member Author

jpfoley commented Sep 12, 2012

Ideally, yes, CG, multi-shift CG and for the clover inverter, which is
BiCGstab right now.

Justin.

On 09/12/2012 10:33 AM, mikeaclark wrote:
For what solvers do you need this? For all of CG, multi-shift CG and
BiCGstab?


Reply to this email directly or view it on GitHub
#80 (comment).

@maddyscientist
Copy link
Member

ok, merging the heavyquark into multishift branch at the moment. Then we can add support for multishift CG and BiCGstab.

@maddyscientist
Copy link
Member

Ok, I have merged the heavyquark branch into the multishift branch. The heavy quark residual stopping condition is now supported in both BiCGstab and CG for whatever fermion action you throw at it. It's not optimal yet because:

  1. The residual calculation is done extremely suboptimally with unnecessary memory copies. This has about a 20% hit in performance over the usual L2 residual computation.
  2. The multi-shift solver isn't supported yet. This is because i'm not sure how to efficiently implement this. In principle one has to recompute the HQ residual for each shift every iteration since now we can't relate the residuals between the shifts. How is this done in MILC? Is this functionality critical?

If sub-performant HQ residual in CG and BiCGstab are good enough, I'm happy to sign off on this and close the issue. We can have a future issue to optimize this.

Refer to commit ff1d197 (ignore the warnings in the commit message, I removed the merge conflicts).

@maddyscientist
Copy link
Member

This is now merged into master with commit 3b21a83.

@maddyscientist
Copy link
Member

This issue is closed by commit 435592b. This is high-performance, and supports all fermion types and solver types. The only exception is the multi-shift solver, which only supports staggered fermions at the refinement stage. Note new the new parameter tol_hq_offset[QUDA_MAX_MULTI_SHIFT] which is used to set the heavy quark residual stopping value for the refinement stage.

@maddyscientist
Copy link
Member

For reference I note that commit f7bd771 added support for refinement for Wilson.

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