-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Treatment of NULLs in ivo_epoch_prop #19
Comments
On Thu, May 09, 2024 at 08:28:52AM -0700, Mark Taylor wrote:
which says _"Zero is used as the default value for any missing null
input parameter"_' though the details are not identical
(`esdc_epoch_prop` yields zero for `parallax` and `rv` outputs
given NULL `parallax`, `pm`, `rv` inputs, while GAVO's
`ivo_epoch_prop` yields NaN for those).
I'd argue NaN (a.k.a. NULL in a VOTable context for doubles) is less
bad here, but...
Do other people agree this looks problematic?
...we are certainly setting up bad traps here. I have missed this
problem, and I would consider it bad enough to change the definition
of ivo_epoch_prop, as in: take it out of the current batch before
others implement the data-inventing behaviour you are seeing.
While inventing data certainly is out of the question, it is unclear
to me what behaviour we should demand instead. I could see returning
NULL rather than the array when the proper motion is missing;
presumably, nobody would want a non-propagated position in a
six-parameter solution, where that's a lot less clear for
ivo_epoch_prop_pos.
However, requring people to pass in a parallax, let alone an RV,
seems wrong even in the age of Gaia. I suppose we *could* make them
NaN on output if they were not passed in, as a special marker that no
distance information is available. It's not pretty, but at least
we'd not be inventing too much data. Hm.
|
Up till now the equivalent functionality in stilts/topcat [Concerning RV, it does the bad thing I'm complaining about above: a NaN input RV turns into a definite output RV.] So I was considering changing its behaviour, or maybe adding another function, that does the following:
I would consider that behaviour a definite improvement on the current behaviour of If you're using this function to get the best available astrometry at a given epoch (apply PM if available, otherwise don't), I'd say this is what's required. My guess is that covers most people's requirements here. There is another usage scenario which only wants positions at a given epoch if they are known to have definite proper motions applied. You could imagine separate UDFs, or a flag on |
So... I have implemented what is proposed in PR #20 in a local pgsphere, or so I think. I will wait for comments here before putting this on the operational server, though, so you cannot try it just yet. Opinions? |
The Gaia epochProp and epochPropErr functions now treat NaN values for parallax, pmra, pmdec and rv as if equal to zero for the propagation itself rather than letting the NaNs propagate through and likely give NaN values for all propagated astrometry parameters. However there is some additional work going on, so that input NaN values of parallax, PM and RV do not lead to definite values in the propagated astrometry. I believe this is smarter than what was done by the esdc_epoch_prop UDFs in place at the Gaia archive, which can turn NULLs (via zeros) into definite values by epoch propagation. Following my lobbying related to this change the ivo_epoch_prop UDF defined in the UDF Catalogue Note is proposed to behave (almost exactly) the same way as the changed behaviour here. See discussion at ivoa-std/udf-catalogue#19 and C9GACS-825.
The description of
ivo_epoch_prop_pos
, which returns a propagated(ra,dec)
position, basically says that for the parametersparallax
,pmra
,pmdec
andradial_velocity
, NULL values must be treated as zero. That makes sense.However,
ivo_epoch_prop
, which returns a propagated(ra,dec,plx,pmra,pmdec,rv)
array, inherits the same parameter descriptions ("This is epoch_prop_pos as described in Sect. 2.2.1, except it returns a full parameter set at out_epoch.") That is problematic, because input NULL values get propagated as output non-NULL values. Try for instance at GAVO DC:All the input
radial_velocity
values are NULL, but the propagated ones (apart fromrandom_index=5
) have a small definite value. Futhermore theparallax
,pmra
andpmdec
values for the source withrandom_index=5
, which are NULL on input, propagate to definite values of zero.I note that this documentation and behaviour more or less follows practice of the Gaia archive, which says "Zero is used as the default value for any missing null input parameter"' though the details are not identical (
esdc_epoch_prop
yields zero forparallax
andrv
outputs given NULLparallax
,pm
,rv
inputs, while GAVO'sivo_epoch_prop
yields NaN for those).Since DPAC has more accomplished astrometers on board than me I wonder whether this behaviour really is respectable... but I have to say it doesn't seem like it.
Do other people agree this looks problematic?
The text was updated successfully, but these errors were encountered: