milc-qcd / milc_qcd

MILC collaboration code for lattice QCD calculations
Other
34 stars 33 forks source link

Feature/gauge action quda #57

Closed weinbe2 closed 1 year ago

weinbe2 commented 1 year ago

This PR adds support for calculating the gauge action, as well as the plaquette and Polyakov loop, to the MILC RHMC/D evolution. It is complimentary to the recently-merged QUDA PR: https://github.com/lattice/quda/pull/1308 . The changes in this PR are enabled via a new MILC Makefile flag, WANT_GA_GPU ("Gauge Action"). This flag has been added to the compile helper script in the ks_imp_rhmc directory. There is also a new complimentary GATIME flag to enable timing the gauge action bits of the RHMD.

The modifications to MILC that enable this PR include:

I also slipped in making it easier to specify the QUDA verbosity level from the Makefile, adding support for a new Makefile variable QUDA_VERBOSITY.

I have confirmed that this code gives results consistent with previous versions of MILC/QUDA, and "does no harm" to a CPU-only build.

This optimization typically leads to an ~5% end-to-end speed-up in a typical RHMC workflow (though this varies as a function of the number of steps per trajectory, as these are once-per-trajectory measurements).

detar commented 1 year ago

Hi Evan, This change is quite extensive. I have a number of questions. (1) Why the "_ks" suffix on various pure gauge routines? Was this because the gauge field includes the KS phases? It seems odd to imply that the pure gauge functions depend on the fermion action. (2) As originally written, and as it is now, there are several choices of improved gauge actions. See generic/imp_actions. This was done originally to allow experimentation. But all of our current running uses the symanzik_1loop_hisq_action.h. Does QUDA support the other actions in generic/imp_actions?

weinbe2 commented 1 year ago

Thanks for the feedback, Carleton! To your questions:

  1. The _ks suffix is indeed because the gauge field includes the KS phases, so it abstracts away needing to apply rephase(OFF/ON) for the CPU while not needing to on the GPU. This was introduced in the gauge residency PR from @mathiaswagner , I believe. That said, I noticed that I had left one *_ks function in the generic directory, void g_measure_ks( ), as opposed to making a separate file in the generic_ks directory. I can address that.
  2. I wrote the gauge action offload routines in a way that's agnostic to the exact gauge action, using get_loop_num(), get_loop_length(), etc, so this part should be fine in QUDA. The other bits (gauge force, stencil, etc) have what's in symanzik_1loop_hisq_action.h "baked into" QUDA (namely in the milc_interface.cpp file), presumably for historical reasons/no one's expressed a need. It could be done, but it would be a non-trivial effort outside the scope of this PR.
weinbe2 commented 1 year ago

I misspoke a bit on the rigidity of the stencil; the HISQ coefficients are pulled in from what's in the action headers, but the only support in QUDA is for HISQ, not standalone ASQTAD, for example.

The Symanzik gauge force paths are baked in here: https://github.com/lattice/quda/blob/develop/lib/milc_interface.cpp#L490 ; but the coefficients are also passed in from MILC, so you do have the flexibility to just have the plaquette term, plaquette + rectangle, or the full plaquette + rectangle + chair.

weinbe2 commented 1 year ago

I pushed some clean-up, plus a fix for the pinned memory leak in the spectrum workflow that @stevengottlieb pointed out to me. Please let me know what you think, @detar . If there are more MILC+QUDA workflows you'd like me to test with this gauge action PR, let me know --- I'd be happy to check anything that'll help push this PR over the finish line.