ratt-ru / codex-africanus

Radio Astronomy Algorithms Library
https://codex-africanus.readthedocs.io
Other
19 stars 10 forks source link

Dictionary used in the convert #310

Open LeoVergaraS opened 2 months ago

LeoVergaraS commented 2 months ago

Description

Hello, I would like to know why .imag and .real are used within the dictionary that convert uses when converting from correlations to Stokes visibilities. The visibilities should be complex numbers, but with this approach, real numbers are obtained.

What I Did

I did the following, is it correct?

"I": {
        ("XX", "YY"): lambda xx, yy: (xx + yy)/2,
        ("LL", "RR"): lambda ll, rr: (ll + rr) /2,
    },
    "Q": {
        ("XX", "YY"): lambda xx, yy: (xx - yy)/2,
        ("RL", "LR"): lambda rl, lr: (rl + lr)/2,
    },
    "U": {
        ("XY", "YX"): lambda xy, yx: (xy + yx)/2,
        ("RL", "LR"): lambda rl, lr: (lr - rl) * 1j /2,
    },
    "V": {
        ("XY", "YX"): lambda xy, yx: (yx - xy) * 1j/2,
        ("LL", "RR"): lambda ll, rr: (rr - ll)/2,
    },
sjperkins commented 2 months ago

It's been a while since I looked at this:

https://github.com/ratt-ru/codex-africanus/blob/cbc64d77517503a4f1b1544242bd6623def1ad8f/africanus/model/coherency/conversion.py#L13-L38

but it's likely because the RR, LL, XX, YY correlations have a zeroed imaginary component with respect to the stokes components.

     "RR": {("I", "V"): lambda i, v: i + v + 0j}, 
     "LL": {("I", "V"): lambda i, v: i - v + 0j}, 
     "XX": {("I", "Q"): lambda i, q: i + q + 0j}, 
     "YY": {("I", "Q"): lambda i, q: i - q + 0j}, 

I now wonder if this is a dangerous assumption to make in terms of real data. @landmanbester @o-smirnov, what do you think?

bennahugo commented 2 months ago

The conversions by @LeoVergaraS are correct. The conversions in this implementation (for I,Q,U,V) are presumably only used once visibilities have been gridded and fourier transformed Complex->Real - otherwise they would not make sense. Mind you stokes products only make sense in image space, not visibility space.

o-smirnov commented 2 months ago

What context is the conversion being done in? "Physical" IQUV is always real so the resulting XX/YY/RR/LL will be real-only. When going from observed coherencies to Stokes, the imaginary part is usually discarded (it is zero by construction assuming perfect visibilities), so the above is correct. In practice, the resulting "imaginary" Stokes will be non-zero due to noise and calibration errors. I could just about conceive of a use case where one wants to retain the imaginaries for diagnostic purposes, but rather cross that bridge when we come to it...

landmanbester commented 2 months ago

We definitely don't expect the Stokes coherencies to be real valued, only the Stokes images are real after gridding+FFT. It looks like this function is meant for visibilities though so I think the implementation is incorrect.

Mind you stokes products only make sense in image space, not visibility space.

Why do you say this? In my mind Stokes coherencies are just what you get if you apply the vCZ theorem to Stokes images. Of course the conversions are more complicated when taking gains into account but that doesn't mean Stokes coherencies don't make sense.

In practice, the resulting "imaginary" Stokes will be non-zero due to noise and calibration errors. I could just about conceive of a use case where one wants to retain the imaginaries for diagnostic purposes, but rather cross that bridge when we come to it...

If the visibilities are assumed to be Hermitian the imaginary parts of the corresponding images have to cancel by construction. Dirty images will of course have an imaginary component if we only grid half the uv-plane (the usual case)

miguelcarcamov commented 2 months ago

Hi, @LeoVergaraS is my undergraduate student and we had this question for a long time. The code in africanus did not make sense to me since the convert function and its dictionary is meant to convert visibilities with shape (row, chan, corr) to visibilities with shape (row, chan, stokes).

There is a confusion in the thread. The context of the conversion is in visibility space and not image space where stokes values are real valued.

sjperkins commented 2 months ago

One solution to the above might be to add entries forIc, Qc, Vc and Uc to the table below, that don't discard the imaginary component. So Ic would mean I complex.

https://github.com/ratt-ru/codex-africanus/blob/cbc64d77517503a4f1b1544242bd6623def1ad8f/africanus/model/coherency/conversion.py#L13-L38

Other suggestions:

Would you consider submitting a PR?