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

New FloatNOrder, gauge fixing and some pure gauge gauge configuration routines #252

Closed
wants to merge 8 commits into from

Conversation

nmrcardoso
Copy link
Contributor

Main changes:
Modified unitarize_links_quda.cu in order to support link unitarization for 12/8 parameters
Modified copy_gauge_extended.cu in order to support copy from extended to regular gauge
Modified gauge_field_order.h, FloatNOrder struct optimized for double and single precision
Added gauge fixing to interface_quda.cpp and milc_interface.cpp
Gauge fixing files:
lib: gauge_fix_ovr_extra.cu, gauge_fix_fft.cu, gauge_fix_ovr_extra.h, gauge_fix_ovr.cu, gauge_fix_ovr_hit_devf.cuh, CUFFT_Plans.h
For pure gauge config generation:
pgauge_exchange.cu, pgauge_init.cu, pgauge_heatbath.cu, pgauge_plaquette.cu, random.cu

@maddyscientist
Copy link
Member

This looks like really nice work Nuno. 👍

Some comments / questions:

  • How much does this add to the compile time and library size? I see you haven't got a configure option to enable compilation and it look like cufft and curand are always lined in. I think it would it be better to enable this through ./configure --enable-gauge-fix or something like that.
  • Does the test program provide testing for all the new functionality?
  • It's great to have heatbath and overrelaxation in QUDA (even if overlapping communication isn't there yet)
  • This needs more documentation, especially in modified and new header files. In particular, most importantly, can you add full doxygen comments to your include/quda.h and include/quda_milc_interface.h.
  • I've started to work on the documentation in the wiki (e.g., https://github.com/lattice/quda/wiki/Updating-the-QUDA-Interface, https://github.com/lattice/quda/wiki/QUDA-multi-GPU-Running-Tips); can you make a wiki page on how to use the gauge fixing and gauge generation algorithms? I'm going to be working on drastically improving the wiki over the next two weeks.

I'll look at the performance of the other kernels in the next day or two (clover, gauge force, copy, etc.) and see how hard it is to update the half precision routines.

@maddyscientist
Copy link
Member

I've just noticed that this pull request is into master and not into develop. All merge work like this should go into develop and not into master directly: master is being kept "golden" and will always correspond to the latest release.

@mathiaswagner
Copy link
Member

Thanks Nuno.
Since your branch is several months old the best strategy (and I guess our preferred way of handling this) is to first merge develop into your branch. This is to reduce possible merge conflicts still in your branch. (and make sure this doesn't break anything).

Then the actual pull request for feature -> develop can be merged without conflicts. This is preferred as otherwise the person handling the pull request needs to deal with the merge conflicts (and no one understands better what has been added to the code then the one who wrote it). Furthermore if merging breaks something this is not yet in develop and can be dealt with in the feature branch before submitting the pull request.

Anyway: great features!

@nmrcardoso
Copy link
Contributor Author

Sorry, I have forgotten that I've used the master branch...
I'll do that and address all suggestions given by Mike.
I've done a case example, which is in tests/su3_testing.cpp, which is based
on su3_test.cpp and also works if compiled for multi-GPU support as the
su3_test.cpp.
This test example have the pure gauge generation and the gauge fixing.
Also, I never did a configure file, if someone can help me with this later
on would me great.
Thanks a lot.

On Thu, May 21, 2015 at 7:21 PM, Mathias Wagner [email protected]
wrote:

Thanks Nuno.
Since your branch is several months old the best strategy (and I guess our
preferred way of handling this) is to first merge develop into your branch.
This is to reduce possible merge conflicts still in your branch. (and make
sure this doesn't break anything).

Then the actual pull request for feature -> develop can be merged without
conflicts. This is preferred as otherwise the person handling the pull
request needs to deal with the merge conflicts (and no one understands
better what has been added to the code then the one who wrote it).
Furthermore if merging breaks something this is not yet in develop and can
be dealt with in the feature branch before submitting the pull request.

Anyway: great features!


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

@maddyscientist
Copy link
Member

Nuno, I'll make a configure file for this, no problem. I've got half-precision support done, just got to do some testing on this before I push it.

How about I make a new branch feature/gauge-fixing and merge in your quda-FloatNOrder branch and the develop branch and then make a new pull request from that?

@nmrcardoso
Copy link
Contributor Author

Thanks a lot Mike, if you could do that it would be great.

On Thu, May 21, 2015 at 8:52 PM, mikeaclark [email protected]
wrote:

Nuno, I'll make a configure file for this, no problem. I've got
half-precision support done, just got to do some testing on this before I
push it.

How about I make a new branch feature/gauge-fixing and merge in your
quda-FloatNOrder branch and the develop branch and then make a new pull
request from that?


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

@mathiaswagner
Copy link
Member

Good plan.

Another question: Alexei mentioned some FFT w/ Multi-GPU issues. Are these solved or what is the status there?

@nmrcardoso
Copy link
Contributor Author

Currently there is no support for gauge fixing with FFTs with multi-GPUs,
its runs if compiled with MPI using only one GPU, if someone tries to use
the gauge fixing with FFTs with more than one GPU it exit the program with
a quda error message.
Only the gauge fixing with overrelaxation supports multi-GPU.
Can you tell which FFT issues was?

On Thu, May 21, 2015 at 9:20 PM, Mathias Wagner [email protected]
wrote:

Good plan.

Another question: Alexei mentioned some FFT w/ Multi-GPU issues. Are these
solved or what is the status there?


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

@mathiaswagner
Copy link
Member

Exactly what you described.
Maybe it is a good idea to create an issue so that we know that this is a missing feature which might be nice to have at some point???

@maddyscientist
Copy link
Member

I've created #253 which supersedes this pull request. Closing this request, any further conversation should be moved there.

@maddyscientist maddyscientist deleted the quda-FloatNOrder branch June 7, 2015 14:40
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.

3 participants