CIRA-Pulsars-and-Transients-Group / vcsbeam

Beamforms MWA-VCS data
Other
3 stars 1 forks source link

Picket-fence calibration interpretation appears incorrect? #36

Closed bwmeyers closed 6 months ago

bwmeyers commented 11 months ago

@cplee1 Has better examples of this than I do, but essentially, when dealing with picket fence data, the way the VCSBeam reports to be interpreting the calibration solutions assumes that the full 30.72 MHz bandwidth is present. It then assumes the calibration solution channel widths based on this information and thus, miscalculates. It's unclear if this is just a reporting issue or whether it really applies much-wider-than-necessary channel solutions to the data...

bwmeyers commented 10 months ago

Status: fixed the calibration interpretation of channel widths, etc. The caveat now being that the number of REQUESTED coarse channels to process ( == MPI world size) must match correspond to the number of coarse channels in your calibration solution. The fine channelisation interpolation is figured out on the fly, but the underlying assumption is that if you want 5 coarse channels processed, the cal. sol. file you provide ONLY has solutions for those 5 channels.

The next issue that seems related is a GPU shared memory alignment. Not sure exactly what's happening there, but may need someone with more CUDA experience to take a look at some point.

bwmeyers commented 10 months ago

I believe I've now fixed the issue. There are strict memory allocation/access requirements for GPU shared-memory (4-byte boundaries). It just so happens that previously, our memory access patterns (with M*nant, where nant=128 or nant=144) did not violate this requirement (given we are access complex-double types, which guaranteed that there is always an access that fit within the 4-byte blocks).

For MWAX data, there is no guarantee that nant is even (e.g., in Dec 2023, the CRAM tile was included, so now there are 145 antenna in an observation). This means we have to actually consider how to access the shared memory arrays, and assigning things based on event indices (i.e., integer numbers of 4-byte chunks since a complex double takes up 16 bytes).

bwmeyers commented 10 months ago

For posterity, I changed these lines

https://github.com/CIRA-Pulsars-and-Transients-Group/vcsbeam/blob/2a93cdfb383905a1cdb9f0a5cdae881ceb0a33e8/src/form_beam.cu#L272-L276

bwmeyers commented 6 months ago

Nominally, this is corrected (combination of PRs #34 #42 #43 #44) as long as the following condition is met: