JeffersonLab / qphix

QCD for Intel Xeon Phi and Xeon processors
http://jeffersonlab.github.io/qphix/
Other
13 stars 11 forks source link

load instructions for TBC phases are always generated for all directions #60

Closed kostrzewa closed 7 years ago

kostrzewa commented 7 years ago

See dslash_body() in code generator. Fix should be easy enough by encapsulation with if(tbc[dim]).

kostrzewa commented 7 years ago

It also seems that the interfaces for the EvenOddLinear operators have not been extended for TBC support.

kostrzewa commented 7 years ago

I'll fix the latter today issue today.

kostrzewa commented 7 years ago

Similarly, the constructor should always take the phases as doubles, will fix.

kostrzewa commented 7 years ago

One more issue: as far as I can tell, TBC multiplications are missing in the face reconstruction.

martin-ueding commented 7 years ago

I'll look into all of these things.

kostrzewa commented 7 years ago

I've fixed the constructors and the eo operator (see cmake-build branch). I haven't touched the other points. Maybe we can take a look together on Monday and derive what is going wrong where from the tmLQCD test.

kostrzewa commented 7 years ago

The CMake-based build works great, by the way, but only using the CLI arguments (as described in the README). For the curses-based interface, we will need to properly define the types of the CMake variables.

Also: Truly excellent work on the integration of the code generator, the jinja template approach as well as the instantiation of all kernels. It makes developing client software much, much nicer. The approach could find an application also in the QUDA build system, which is similarly bogged down by template instantiation.

bjoo commented 7 years ago

Thanks very much! I am also liking CMake a lot now, tho I am still a novice. (E.g. I didn’t know how to type the options) I’ll get to it eventually :).

Another thing is that it would be useful to export the public targets to make QPhiX easy to use for follow on projects which use CMake.

Having the generated code in a library is helpful for this: only 1 lib to export, transitively. Also the issue is the header files. However, the number of these is also now bounded :)

Many many thanks go to Martin for all the hard work he has done !!!!


Dr Balint Joo High Performance Computational Scientist Jefferson Lab 12000 Jefferson Ave, Suite 3, MS 12B2, Room F217, Newport News, VA 23606, USA Tel: +1-757-269-5339, Fax: +1-757-269-5427 email: bjoo@jlab.org

martin-ueding commented 7 years ago

Thank you for the kind words! It is good to hear that you like the changes and that they make developing easier.

The CMake-all-way changes are very nice, I like the compact colored output 😄.

The face reconstruction should now also perform the phase multiplication. The loading of the phase is now guarded in the code generator, it should only be used when needed. For the other cases, there are unused variables now. Those should not have a performance impact, they should be removed from the function scope.

That leaves work on the QPhiX side, the proper face reconstruction kernels have to be called and the arguments have to be passed through. But not today 😉

martin-ueding commented 7 years ago

I think that last task has been done in 98a0a025df7bff636f4a1ef6b484dcd9cb03077e, right? So the twisted boundary conditions are implemented now?

kostrzewa commented 7 years ago

I agree, let's close this :D Thanks for adding also the NDTM tbc's.

kostrzewa commented 7 years ago

ps: there's still the tbc declaration, which is always generated, but maybe this is innocuous

martin-ueding commented 7 years ago

I'd hope that the compiler would remove such an unused variable, even though it gets initialized. It might be fixable by using a two-step declaration in the code generator and only do the second step if one really needs that. I might do that after I have done the changes to the mass preconditioning.