Update CASA sdintimaging#347
Conversation
This PR syncs our bespoke sdintimaging task to the latest CASA one. There have been improvements here, but the underlying non-convergence is still an issue. We now will automatically set the dishdia based on beam and frequency, gain can be supplied as a parameter in .clean files, or falls to a default of 1. Also takes the chance to make the sd prepping a function to reuse code between the imaging handler and the chunked imaging handler. Note this doesn't have keyHandler changes for inputting gain on a per-obs basis. I think this PR is already big enough - Update taskSDIntImaging - General tidy-up of sdintimaging logic - Updated changelog
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #347 +/- ##
=========================================
+ Coverage 6.27% 6.32% +0.05%
=========================================
Files 38 38
Lines 15229 15244 +15
Branches 3653 3656 +3
=========================================
+ Hits 955 964 +9
- Misses 14260 14266 +6
Partials 14 14 ☔ View full report in Codecov by Sentry. |
| # Convert through to dish diameter in m | ||
| lam = 3e8 / freq.to(u.Hz).value | ||
|
|
||
| dishdia = 1.22 * lam / restoring_beam |
There was a problem hiding this comment.
Is 1.22 the right prefactor for ALMA? I know it's not for the GBT, for example. It seems like it should usually be 10.7m (https://help.almascience.org/kb/articles/what-primary-beam-does-casa-use-for-alma-12-m-antennas-and-what-is-the-actual-alma-12-m-antenn)
There was a problem hiding this comment.
do these change between telescopes? It would be good if this could be pulled out of the header, but I'm not sure it's always recorded. Happy to poke on this to see if it actually gets recorded, or just change the number
There was a problem hiding this comment.
1.22 is what CASA seems to use under the hood in feather (https://git.astron.nl/tol/casa/-/blob/master/casa5/code/synthesis/MeasurementEquations/Feather.cc#L247). Since that's what sdintimaging is doing, it's maybe best to keep this consistent (even if wrong?)
There was a problem hiding this comment.
That was my first question, namely how is it used in the feather. I was digging through the code but you found it first. Second question is more curiosity, namely what does this give for an ALMA 12m dish in practice? If it's kicking back 12m with the 1.22, that's definitely not the right thing to go into feather and we may need to be more specific about it.
There was a problem hiding this comment.
I saw it give 11.6m for my test case. The PSF here just determines the weighting for the Fourier combination in the uv overlap region though, right? So getting it a bit wrong isn't catastrophic, it just ends up being a non-optimal combination. Though I don't think we ever pass the number through in the feathering, so we've never actually got this right
This PR syncs our bespoke sdintimaging task to the latest CASA one. There have been improvements here, but the underlying non-convergence is still an issue.
We now will automatically set the dishdia based on beam and frequency, gain can be supplied as a parameter in .clean files, or falls to a default of 1.
Also takes the chance to make the sd prepping a function to reuse code between the imaging handler and the chunked imaging handler.
Note this doesn't have keyHandler changes for inputting gain on a per-obs basis. I think this PR is already big enough