askap-vast / vast-pipeline

This repository holds the code of the Radio Transient detection pipeline for the VAST project.
https://vast-survey.org/vast-pipeline/
MIT License
8 stars 3 forks source link

User systematic errors not implemented as intended #470

Open ajstewart opened 3 years ago

ajstewart commented 3 years ago

In writing the documentation I noticed that the implementation of the user supplied systematic positional errors is not really as it should be.

I'm referring to the options in the pipeline run config file:

ASTROMETRIC_UNCERTAINTY_RA = 1 # arcsec
ASTROMETRIC_UNCERTAINTY_DEC = 1  # arcsec

These should be implemented on a per pipeline run basis - which is what the intention here is given that these are set in the pipeline run config.

However when going through the code these numbers are actually assigned, and hence fixed, to the measurements in the image ingest stage. Therefore, when these measurements are used again by another user in their own pipeline run, the systematic uncertainty has already been hard fixed and their value won't have any affect.

https://github.com/askap-vast/vast-pipeline/blob/66b2dcc54ebcb2e125bf39b9c1f2232f2b674697/vast_pipeline/image/main.py#L283-L302

The solution is not that straight forward as you would have to change some of the database because these uncertainty values should still be saved. I.e. you'd have to set up a new table with uncertainty values related to a measurement for that specific run. These would also have to be calculated at the beginning of every job.

This is not a huge error, for now I would recommend every user be keeping the uncertainty to the default value of 1 arcsec. At least then everything will be consistent. It also only really impacts the de ruiter association method.

taramurphy commented 3 years ago

@ajstewart and @marxide : we are discussing whether these names:

ASTROMETRIC_UNCERTAINTY_RA = 1 # arcsec ASTROMETRIC_UNCERTAINTY_DEC = 1 # arcsec

are correctly representing what these parameters mean. Can you explain in plain English what they are. We are wondering if "Systematic offset" or something is more correct?

ajstewart commented 3 years ago

@ajstewart and @marxide : we are discussing whether these names:

ASTROMETRIC_UNCERTAINTY_RA = 1 # arcsec ASTROMETRIC_UNCERTAINTY_DEC = 1 # arcsec

are correctly representing what these parameters mean. Can you explain in plain English what they are. We are wondering if "Systematic offset" or something is more correct?

Yes it's a systematic error system - these parameters enable the user to add (in quadrature) to the error of the selavy measurements, originally intended to be on an individual run basis as I say above. Ends up only being used when the de Ruiter method is used - basic and advanced association don't use the positional uncertainties.