Ilbelov 3pi fits - #417
Ilbelov 3pi fits#417iljatt wants to merge 5 commits into
Conversation
|
Build status for this pull request: FAILURE Build log: make_ilbelov_3pi_fits.log |
|
Build status for this pull request: FAILURE Build log: make_ilbelov_3pi_fits.log |
|
Test status for this pull request: SUCCESS Summary: summary.txt Build log: make_ilbelov_3pi_fits.log |
|
This is a good strategy for handling the polarization in merged trees. However, it is worth noting at the convention is different than the strategy used elsewhere. Fore example see line 87 of Zlm.cc -- here the 3 elements of the beam four-vector are used to hold elements of a vector that points in the direction of the polarization with a magnitude equal to the magnitude of the polarization: ( P cos( polAngle ), P sin( polAngle ), 0 ). It might be helpful to standardize the approach across amplitude classes to avoid confusion. Also note that for fits with large numbers of amplitudes, this "division of labor" between calcUserVars and calcAmplitude puts a lot of load on the calcUserVars method. If you like doing GPU fits, this may leave the GPU under-utilized because user variables are all computed on the CPU. In general there is no "one-size fits all" optimization here because it depends on the hardware you are using, where free parameters show up, etc. I don't consider this an issue or problem that needs resolution before merging, I just wanted to point it out. |
|
I understand also that implementing my comment above requires reformatting root trees. Perhaps this is a pain right now and there is urgency just to get the amplitude in the main code base so that MC studies could be done with the OSG framework. If that is the case - no problem. I can approve this and we can put unifying the beam polarization representation as a "to-do" for the future. |
|
Many thanks for review and your comments, Matt. I think, we can postpone merging to the master branch then. I will reformat my trees and update the amplitudes. Then I will also have to update the decayAngles amplitude, I would like to overload the getPhiProd() function with analogous function which receives epsilon-vector instead of polAngle. |
|
OK - I wasn't sure if you really needed this amplitude and its registration in a production release to be able to efficiently use it. Simultaneously @amschertz is working on a revisions of probably what are very similar decay angles codes for the b1 fitting. It might be good to try to coordinate this a bit. Amy had a few goals in her revisions aimed at performance optimizations. For her fits, going to the GPU is not beneficial for the reasons I noted above. We were trying to make that a bit more flexible. |
|
The thing is that I didn't compare efficiency for this amplitude between running fits on CPUs and on GPU. |
|
Perhaps, we will have to figure out if we can benefit from switching to GPUs on our cluster. |
|
I saw similar issues with Amy's production fits on GPUs. I just sent you by email some notes that I posted to slack about a month ago. If your fit time is spent in calcUserVars then GPU is not going to help since that is done on CPU by design. To make GPU work efficiently you might structure the fit so user variables are "static" (they can be reused by every instance of the amplitude). That shifts compute load to the calcAmplitude, which is parallelized on GPU. The MPI implementation will parallelize everything, including calcUserVars. GPU is probably best if you are floating parameters in amplitudes in the fit. I'm happy to help with optimizing as I can. I think one thing that would be helpful is to put some timers in the fitting function. There are generally 3 blocks that you might be interested in timing:
For Amy's production fit, the most expensive of these was (3). That step is accelerated by MPI but not significantly by GPU. So, one needs to look at that computation and how to do it most efficiently. Let me know if I can help more. |
|
It might be worth considering running GPU jobs at the JLab Farm. It offers in total 28 A800 cards and Alex could arrange prioritized access. |
Amplitudes will work on trees with merged diamond orientations. The two necessary parameters (polarization fraction and diamond orientation angle) are either defined in the config file and passed to the amplitude as external parameters, or read from the input trees as px and py components of the beam four-vector.