epic-astronomy / LWA_EPIC

The LWA-specific implementation of the EPIC correlator
http://livetv.epic-astronomy.org
MIT License
3 stars 3 forks source link

Duplicated code #9

Open mkolopanis opened 4 years ago

mkolopanis commented 4 years ago

LWA_bifrost.py and LWA_bifrost_DFT.py have lots of duplicated code. Some questions we should consider

jaycedowell commented 4 years ago

We might be able to address some of this with judicious use of bifrost.pipeline. Here is an example of what the interface is like but I personally haven't used it.

mkolopanis commented 4 years ago

I've been looking more into cleaning up the two scripts I mentioned. I've found the following major difference between the MOFF correlator implementations. When implementing the romein kernel, the non-dft version of the code uses polmajor=False in the two instances polmajor appears as a function kwarg. The DFT version always uses polmajor=True though.

Reading through the kernel source code, setting polmajor=True basically forces a single polarization. Is this the expected behavior @KentJames? Or maybe the code split off and at some point polmajor was set to false in the non-DFT but never updated in the DFT script?

I'm looking to combine these into just one script. They are nearly identical and the DFT one only seems to add functionality except for the polmajor keyword (and a reordering of axes which I don't think should be a major issue).

https://github.com/epic-astronomy/LWA_EPIC/blob/f22d93364ebda98ab476483fc95f7bcac6019696/LWA/LWA_bifrost.py#L939-L945

https://github.com/epic-astronomy/LWA_EPIC/blob/f22d93364ebda98ab476483fc95f7bcac6019696/LWA/LWA_bifrost_DFT.py#L1084-L1090

https://github.com/epic-astronomy/bifrost/blob/34a0671fc31a417b9266dd07eef566f37d7ecb96/src/romein.cu#L251-L273

current implementation when the link was taken:

template<typename InType, typename OutType>
inline void launch_romein_kernel(int      nbaseline,
                                 int      npol,
                                 bool     polmajor,
                                 int      maxsupport, 
                                 int      gridsize, 
                                 int      nbatch,
                                 int*     xpos,
                                 int*     ypos,
                                 int*     zpos,
                                 OutType* kernels,
                                 InType*  d_in,
                                 OutType* d_out,
                                 cudaStream_t stream=0) {
    //cout << "LAUNCH for " << nelement << endl;
    dim3 block(8,1);
    dim3 grid(nbatch*npol,1);
    if( polmajor ) {
        npol = 1;
    } else {
        block.y = npol;
        grid.x = nbatch;
    }
mkolopanis commented 4 years ago

looking at the git blame on LWA_bifrost.py, the polmajor=False was set after the DFT code was created. Perhaps the update just never flowed through.

mkolopanis commented 4 years ago

looks like this may also be the case with the antenna dimension ordering?