Hybrid map maker #65
Merged
Hybrid map maker #65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrs65
force-pushed
the
hybrid-beamform
branch
from
68ea15c
to
2bee41c
Compare
jrs65
force-pushed
the
hybrid-beamform
branch
from
2bee41c
to
4a7ac96
Compare
jrs65
force-pushed
the
hybrid-beamform
branch
2 times, most recently
from
56523cd
to
7d64211
Compare
tristpinsm
requested changes
Jan 12, 2021
This defines new `FreqContainer` and `SiderealContainer` base containers to support a common set of functionality for data with a frequency or RA axis. It also ports all the existing containers to use them.
… in single_beam mode
This adds datasets needed for tracking the noise and dirty beams of the ringmaps.
jrs65
force-pushed
the
hybrid-beamform
branch
from
447bbdc
to
62dcffb
Compare
|
This pull request introduces 5 alerts when merging 62dcffb into 89b0947 - view on LGTM.com new alerts:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is now ready for review. There's a few broader fixes in here, but the biggest change is the introduction of the componentised ring map maker.
I have tested this against the current ring map maker and it gives nearly identical maps, that differ at the 10^-3 level. Here is a comparison of the maps using uniform weighting:
The top panel is the map from the new map maker (from -10 Jy to 10 Jy), the middle panel is the difference between the old and new maps with the colour scale multiplied by 1000x (i.e. it's from +/- 10 mJy). The third is the difference if I apply a ~10^-3 level scale to the new map to get them to match better.
This is the same comparison but for natural weighting
One obvious question is: why is there any difference at all? Essentially it all comes down to how the different maps are renormalised for missing baselines. The original map maker constructs the weights for all baselines initially, and normalises by the sum of all the weights to guarantee that the output units are faithfully Jy / beam (i.e. if you take the pixel containing CygA it will have the value of the flux of CygA). The new map maker first constructs a grid of NS beams, and each column of EW visibilities is normalised independently. This guarantees that the hybrid visibilities (i.e. localised NS beam, fourier mode EW) which come out of the BeamformNS stage are in units of Jy. However, because the EW transform is done separately it's tough (and not clearly desirable) for this to give each cylinder separation the same relative weighting that they are given in the old map maker.
If the missing NS baselines were identical for each cylinder separation the map makers would give the same result. However, when the missing NS baselines are different the map makers will give different results. The biggest thing which breaks this is the masking of the autocorrelations, which is why the differences are at the ~1/1024 level.
Given that we never really sorted out the most theoretically desirable scheme for the old map maker, I don't think this difference is very problematic, but it would be good to hear if any one else (e.g. @ashill and @ssiegelx who aren't tagged for review) can think of any objections.