-
Notifications
You must be signed in to change notification settings - Fork 110
A new gauge fixing algorithm which returns the rotation field. #1481
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
base: develop
Are you sure you want to change the base?
Conversation
I'm wondering if I should just override the old interface I tested the performance and didn't see any significant performance regression compared to the old implementation. But more testing is definitely needed. |
@Jenkins test this please |
Thanks for this PR @SaltyChiang. I don't think there should be any performance change, so I think we could just replace the old interface with the new one. Can you go ahead and make this change to your PR? I think you'll need to update the MILC interface code (milc_interface.cpp) since that code calls the older interface, but that should be an easy change. |
Remove `spinorRotate`.
Some additional explanation about the distance preconditioning: |
The performance could be further improved by hiding communication time during computation, and the old version of gauge fixing divided the points into two parts called "Border" and "Int" to implement it. The current performance of the gauge fixing cannot beat the old one in some situations (for example, reunit_interval not very small), so I decided not to remove the existing algorithm. I did not write a kernel similar to the old one, since it's not good for readability. I think a similar optimization could be applied to the new implementation by using a special dslash kernel working on a special spinor with Ns=3 and Nc=3. I want to work on other topics and will leave the code here for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all this work @SaltyChiang. This looks like a great contribution.
The main request I have for you is that we would need some unit testing added for these new features.
- For the new gauge fixing functionality, could you add a test for this in the
gauge_alg_test
? - Do you have any thoughts on how to test the shift-only covariant derivative?
include/quda.h
Outdated
@@ -140,6 +142,8 @@ extern "C" { | |||
|
|||
int laplace3D; /**< omit this direction from laplace operator: x,y,z,t -> 0,1,2,3 (-1 is full 4D) */ | |||
int covdev_mu; /**< Apply forward/backward covariant derivative in direction mu(mu<=3)/mu-4(mu>3) */ | |||
bool covdev_shift; /**< Apply the shift instead of the covariant derivative */ | |||
bool staggered; /**< If the input field is staggered or not for Laplace and CovDev */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a more descriptive variable name is needed here, instead of just staggered
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both COVDEV and LAPLACE will use it, and I thought something like covdev_laplace_staggered
/covdev_laplace_nspin
looks terrible. Another choice is to use two variables like covdev_nspin
and laplace_nspin
. Which one do you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use laplace_nspin
and covdev_nspin
instead. They are initialized as 1 and 4 respectively to keep the former behavior.
Yes, the old version of the code that overlaps comms and compute, while efficient, is horrible to read. Fine to have both versions of the code for now, and for correctness testing, having the two versions is not a bad thing anyway. 😄 |
I noticed the name |
Add init/check/print for `QudaGaugeFixParam`.
@maddyscientist Tests for new gauge fixing and shift-only covdev are added.
|
I implemented a new over-relaxation gauge-fixing algorithm. The major difference between the new and the old implementations is we can obtain the rotation field now. Our workflow often requires this.
The rotation field$g(x)$ is defined as follows:
$$U^\prime_\mu(x)=g(x)U_\mu(x)g^\dagger(x+\hat{\mu})$$
gaugeRotate
to rotate a gauge fieldperformGaugeRotateQuda
in quda.hgaugeFixingQuality
to evaluate gauge fixing qualityExposegaugeFixingQuality
in quda.hQudaGaugeFixParam
to handle parameters for gauge fixingperformGaugeFixQuda
in quda.hAddgaugeShift
to shift a gauge fieldExposegaugeShift
in quda.hThe same feature as Adding gauge shift #1348covdev_shift
toQudaInvertParam
to use the shift kernelQUDA_COVDEV_DSLASH
andQUDA_LAPLACE_DSLASH
staggered
toQudaInvertParam
, which is only used withQUDA_COVDEV_DSLASH
andQUDA_LAPLACE_DSLASH