Feature/quark smearing.pr

alexstrel commented 2 years ago

PR introduces an implementation of the Gaussian smearing for staggered quarks. It also includes computation of two link field, simple test instruction: mpirun -np 1 ./tests/staggered_gsmear_test --dslash-type asqtad --prec double --gridsize 1 1 1 1 --dim 16 16 16 48 --load-gauge path-togauge-config

maddyscientist commented 1 year ago

Thanks @alexstrel for the updates to the PR. I left some more minor comments. Beyond these, the main outstanding issue is the lack of including the test in the ctest list.

weinbe2 commented 1 year ago

@alexstrel do you have a MILC workflow leveraging this that you can share? It can be a dummy one, no need to share anything special. As part of testing on my end it'd just be nice to see it "in action".

hwancheolJeong commented 1 year ago

@alexstrel do you have a MILC workflow leveraging this that you can share? It can be a dummy one, no need to share anything special. As part of testing on my end it'd just be nice to see it "in action".

@weinbe2 Here is my test app: https://github.com/hwancheolJeong/milc_qcd/tree/feature/gaussianSmearing/ks_gauss_smear It is my fork of MILC code. It uses an interface defined in generic_ks/gauss_smear_ks_QUDA.c that will be merged into develop branch later (once this pr is done.), but this test app is solely for personal use. You can build the test app by "make ks_gauss_smear", and run it with an input file (refer to test/ks_gauss_smear.2.sample-in). It will produce a smeared source in Scidac format as well as binary format (for easy comparison).

maddyscientist commented 1 year ago

Thanks for the latest updates @alexstrel and @hwancheolJeong. I've done some tests with the latest version, and things are mostly good. A few outstanding issues before we can merge:

the automatic ctest only runs in single precision, can you add a double precision version as well? (see what is done with gauge_path_test for example for how to deal with this, where it is run in a loop over enabled precision)
reconstruct options seem to be ignored by the test, and the kernel is always run with no reconstruct
can you merge in #1345 into your PR? This will be merged shortly into develop, but in the meantime it will fix the presently broken Jenkins build with CUDA 12 / GCC-12 / Clang 14

There's some other cleanup that I will apply, but this will be for after the branch is merged.

maddyscientist commented 1 year ago

....and lastly, this PR needs a clang-format.

weinbe2 commented 1 year ago

@alexstrel and @hwancheolJeong , this is all really nice! I'm doing some testing and a little bit of cleanup here and there, but this should be good to go soon.

I had one question --- I noticed that you created a routine in milc_interface.cpp, exposed in the appropriate header, to free the two link gauge field, but no routines to create the field or apply the smearing. Is this intentional?

hwancheolJeong commented 1 year ago

Thank you @alexstrel and @hwancheolJeong for continuing to get this PR in good shape. I'm finally approving 😉

@weinbe2 can you take this PR for a spin before we merge?

@maddyscientist I appreciate you! for reviewing it and for your advice and help.

hwancheolJeong commented 1 year ago

@alexstrel and @hwancheolJeong , this is all really nice! I'm doing some testing and a little bit of cleanup here and there, but this should be good to go soon.

I had one question --- I noticed that you created a routine in milc_interface.cpp, exposed in the appropriate header, to free the two link gauge field, but no routines to create the field or apply the smearing. Is this intentional?

Thanks, @weinbe2 . I made an interface inside the MILC code: https://github.com/hwancheolJeong/milc_qcd/blob/feature/gaussianSmearing/generic_ks/gauss_smear_ks_QUDA.c, which will be merged into the develop branch of the MILC code once this PR is done (I may need to update a little to accommodate recent updates in QUDA). We may add a similar interface routine in milc_interface.cpp later.

weinbe2 commented 1 year ago

@alexstrel and @hwancheolJeong , this is all really nice! I'm doing some testing and a little bit of cleanup here and there, but this should be good to go soon. I had one question --- I noticed that you created a routine in milc_interface.cpp, exposed in the appropriate header, to free the two link gauge field, but no routines to create the field or apply the smearing. Is this intentional?

Thanks, @weinbe2 . I made an interface inside the MILC code: https://github.com/hwancheolJeong/milc_qcd/blob/feature/gaussianSmearing/generic_ks/gauss_smear_ks_QUDA.c, which will be merged into the develop branch of the MILC code once this PR is done (I may need to update a little to accommodate recent updates in QUDA). We may add a similar interface routine in milc_interface.cpp later.

Understood, thank you for clarifying. That's not the typical approach but I'm certainly not going to hold up the PR on this! Similar to my other ask, can you please file an issue noting that this needs to be ported into the MILC Interface in the future?

weinbe2 commented 1 year ago

I'll merge this after Jenkins completes

lattice / quda

Feature/quark smearing.pr #1312