Skip to content

Update CASA sdintimaging#347

Open
thomaswilliamsastro wants to merge 1 commit into
masterfrom
update-sdintimaging
Open

Update CASA sdintimaging#347
thomaswilliamsastro wants to merge 1 commit into
masterfrom
update-sdintimaging

Conversation

@thomaswilliamsastro
Copy link
Copy Markdown
Collaborator

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

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
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

❌ Patch coverage is 5.74163% with 394 lines in your changes missing coverage. Please review.
✅ Project coverage is 6.32%. Comparing base (9997b7b) to head (20b6597).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
phangsPipeline/taskSDIntImaging.py 6.15% 244 Missing ⚠️
phangsPipeline/utilsSingleDish.py 8.82% 62 Missing ⚠️
phangsPipeline/handlerImagingChunked.py 2.43% 40 Missing ⚠️
phangsPipeline/handlerImaging.py 2.77% 35 Missing ⚠️
phangsPipeline/casaImagingRoutines.py 0.00% 13 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

# Convert through to dish diameter in m
lam = 3e8 / freq.to(u.Hz).value

dishdia = 1.22 * lam / restoring_beam
Copy link
Copy Markdown
Collaborator

@low-sky low-sky May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

@thomaswilliamsastro thomaswilliamsastro May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants