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

Modifications to add use_smeared_gauge to InvertParam #1522

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

cjmorningstar10
Copy link
Contributor

@cjmorningstar10 cjmorningstar10 commented Dec 10, 2024

Added use_smeared_gauge to QudaInvertParam to get rid of awkward extra bool in some routines. In setDiracEigParam(...), added code to allow this to run without needing gaugePrecise having already been set. Some first tests have been done, but would like feedback on how this was done! These changes facilitate computation of the LapH eigenvectors using the smeared gauge field.

@cjmorningstar10 cjmorningstar10 requested a review from a team as a code owner December 10, 2024 19:26
Copy link
Member

@maddyscientist maddyscientist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cjmorningstar10 do these changes fix the issues that were being seen?

include/quda.h Outdated
@@ -507,7 +511,7 @@ extern "C" {

/** Whether to use the smeared gauge field for the Dirac operator
for whose eigenvalues are are computing. */
bool use_smeared_gauge;
QudaBoolean use_smeared_gauge;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change to QudaBoolean from bool? I intentionally used bool with this variable as we intend to deprecate QudaBoolean.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I think with this addition of the use_smeared_gauge parameter to QudaInvertParm the QudaEigParam variant is never used now? Can we just delete it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used QudaBoolean because all the other boolean members did...it looked weird to see a difference. But bool is fine. The use_smeared_gauge is needed in the InvertParam because only that gets sent to the setDiracParam functions, where it is needed. It is ok to keep it in the EigParam too. I think I put in a check that these two bools have to be the same.

@maddyscientist
Copy link
Member

cscs-ci run

@cjmorningstar10
Copy link
Contributor Author

@cjmorningstar10 do these changes fix the issues that were being seen?

No. The main reason for this PR is to enable computing the LapH eigenvectors if you read the smeared gauge and NOT the original gauge. Once the perambulators are computed, the original gauge field is never needed any more. When we start evaluating correlator functions, we will just read the smeared gauge, compute the Laph evs on the fly, then start computing baryon triplets/meson doublets/ then the correlators. Even displacements in the operators need only the smeared gauge field. No need to read the original gauge, smear it, then .... Just cuts out one step. No need to waste device mem on the original gauge. This is a minor issue, but .... why not support this choice?

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.

2 participants