ska-sa / katsdpcal

MeerKAT calibration pipeline
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Flux density #25

Closed KimMcAlpine closed 4 years ago

KimMcAlpine commented 4 years ago

This PR allows flux density models of sources in calibrator fields to be specified using the wsclean polynomial format (https://sourceforge.net/p/wsclean/wiki/ComponentList/). This is to support more complex models of calibrator fields in the future as discussed in https://skaafrica.atlassian.net/browse/MKAIV-1531.

I have not included the new sky models in this PR as these are still being developed but ultimately there would be a new UHF band model for J0408-6545.

ludwigschwardt commented 4 years ago

General comment: this flux density model should really go into katpoint (eventually 😄) to benefit everyone.

ludwigschwardt commented 4 years ago

I also wondered whether the description string should be closer to the actual WSClean format, i.e. something like

856.0 1712.0 wsclean 0.000748810650400475 [-0.00695379313004673 -0.0849693907803257] false 125584411.621094

Note that ord and log are then unified and the reference frequency is in Hz, so the format is just wsclean.

There is the discrepancy with min/max freq in MHz... Although they could also be in Hz if wsclean moves to the front of the description string.

The main advantage of matching formats is that it would be obvious at a glance that they are equal. This might still be true if I and the coefficients are unaltered, so maybe not a big deal after all. And you still need to drop the commas, so some conversion is required.

What do you think?

KimMcAlpine commented 4 years ago

I also wondered whether the description string should be closer to the actual WSClean format, i.e. something like

856.0 1712.0 wsclean 0.000748810650400475 [-0.00695379313004673 -0.0849693907803257] false 125584411.621094

Note that ord and log are then unified and the reference frequency is in Hz, so the format is just wsclean.

There is the discrepancy with min/max freq in MHz... Although they could also be in Hz if wsclean moves to the front of the description string.

The main advantage of matching formats is that it would be obvious at a glance that they are equal. This might still be true if I and the coefficients are unaltered, so maybe not a big deal after all. And you still need to drop the commas, so some conversion is required.

What do you think?

I'm wondering about what to do then for the case where people supply parameters and not a description string ? Would we abandon that as an option for the wsclean case ?

Also I'm a little bit uncertain of how to tackle to question of different frequency units for the range and the model. Which units would the frequencies you supply to the flux density function be in ? If it's the model 'type' units that could be tricky because the wsclean format units only really depends on the units of the reference frequency - so if you give a reference frequency in MHz and supply frequencies in MHz you might expect that the flux_densities would be correct, but if internally you're expecting the frequencies to be supplied in Hz, then the valid frequency range might not match.

Also I know that Ben wants to mix types of models in the complex models, ie add a baars type model for one source to a wsclean model for another source and then its tricky if I have to supply the two different models with frequencies in different units ?

How to deal with this. Do you add another parameter that indicates the units of the frequencies that the calculation should be done in ?

bennahugo commented 4 years ago

Hi Kim

The WSClean spec allows for log polys normal polys. I wanted to use the central component for specifying e.g. the original component, but from my analysis the fits in ordinary space is pretty good .

On Mon, 20 Apr 2020, 11:42 KimMcAlpine, notifications@github.com wrote:

I also wondered whether the description string should be closer to the actual WSClean format, i.e. something like

856.0 1712.0 wsclean 0.000748810650400475 [-0.00695379313004673 -0.0849693907803257] false 125584411.621094

Note that ord and log are then unified and the reference frequency is in Hz, so the format is just wsclean.

There is the discrepancy with min/max freq in MHz... Although they could also be in Hz if wsclean moves to the front of the description string.

The main advantage of matching formats is that it would be obvious at a glance that they are equal. This might still be true if I and the coefficients are unaltered, so maybe not a big deal after all. And you still need to drop the commas, so some conversion is required.

What do you think?

I'm wondering about what to do then for the case where people supply parameters and not a description string ? Would we abandon that as an option for the wsclean case ?

Also I'm a little bit uncertain of how to tackle to question of different frequency units for the range and the model. Which units would the frequencies you supply to the flux density function be in ? If it's the model 'type' units that could be tricky because the wsclean format units only really depends on the units of the reference frequency - so if you give a reference frequency in MHz and supply frequencies in MHz you might expect that the flux_densities would be correct, but if internally you're expecting the frequencies to be supplied in Hz, then the valid frequency range might not match.

Also I know that Ben wants to mix types of models in the complex models, ie add a baars type model for one source to a wsclean model for another source and then its tricky if I have to supply the two different models with frequencies in different units ?

How to deal with this. Do you add another parameter that indicates the units of the frequencies that the calculation should be done in ?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/ska-sa/katsdpcal/pull/25#issuecomment-616433703, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6QRVYEU2EEWTKSYU3DRNQKHBANCNFSM4MHSSSGQ .

KimMcAlpine commented 4 years ago

As we discussed I've changed the description string to be closer to the WSClean format, so now it accepts strings like: 856.0 1712.0 wsclean 0.000748810650400475 [-0.00695379313004673 -0.0849693907803257] false 125584411.62109

The reference frequency is supplied in Hz, but the min and max frequencies are supplied in MHz, and the frequencies supplied to the FluxModel.flux_density function are still in MHz. This simplifies my life a bit in cal, because I don't have to check the model type before deciding on the units that should be supplied to the function.

I hope this hack will suffice for now, as we discussed the long-term solution to the problem of unit ambiguity is to use astropy in katpoint as astropy has built-in unit awareness.

I have also allowed a user to instantiate the model using parameters.

I also wondered whether the description string should be closer to the actual WSClean format, i.e. something like

856.0 1712.0 wsclean 0.000748810650400475 [-0.00695379313004673 -0.0849693907803257] false 125584411.621094

Note that ord and log are then unified and the reference frequency is in Hz, so the format is just wsclean.

There is the discrepancy with min/max freq in MHz... Although they could also be in Hz if wsclean moves to the front of the description string.

The main advantage of matching formats is that it would be obvious at a glance that they are equal. This might still be true if I and the coefficients are unaltered, so maybe not a big deal after all. And you still need to drop the commas, so some conversion is required.

What do you think?