Skip to content
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

Open
mbtaylor opened this issue May 9, 2024 · 3 comments
Open

Treatment of NULLs in ivo_epoch_prop #19

mbtaylor opened this issue May 9, 2024 · 3 comments

Comments

@mbtaylor
Copy link
Member

mbtaylor commented May 9, 2024

The description of ivo_epoch_prop_pos, which returns a propagated (ra,dec) position, basically says that for the parameters parallax, pmra, pmdec and radial_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:

   SELECT TOP 10
          random_index,
          ra, dec, parallax, pmra, pmdec, radial_velocity,
          ivo_epoch_prop(ra, dec, parallax, pmra, pmdec, radial_velocity, 2016, 2100) AS astrom2100
   FROM gaia.dr3lite
   ORDER BY random_index

All the input radial_velocity values are NULL, but the propagated ones (apart from random_index=5) have a small definite value. Futhermore the parallax, pmra and pmdec values for the source with random_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 for parallax and rv outputs given NULL parallax, pm, rv inputs, while GAVO's ivo_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?

@msdemlei
Copy link
Collaborator

msdemlei commented May 13, 2024 via email

@mbtaylor
Copy link
Member Author

Up till now the equivalent functionality in stilts/topcat epochProp/epochPropErr has propagated blank (NaN) values for parallax or (either) PM through the equations without special measures; the effect is that blank input parallax or PM gives blank values for propagated position, parallax and PM. This is respectable but kind of annoying if you're just trying to get the best estimate for position at some different epoch.

[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:

  • on input, check and record PM, parallax and RV for NULL
  • perform calculations as if NULL values for PM, parallax, RV are zero
  • on output, for each of PM, parallax, RV, if the input was NULL, set the output to NULL

I would consider that behaviour a definite improvement on the current behaviour of ivo_epoch_prop.

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 epoch_prop_pos/epoch_prop that selects this behaviour. However I find it a bit hard to imagine why you'd want this without also wanting the propagated positional errors, which are anyway not supplied by either of these UDFs. So my feeling is that such a requirement is niche enough that we could expect people to handle it manually (WHERE pmra IS NOT NULL AND pmdec IS NOT NULL [AND parallax IS NOT NULL]).

@msdemlei
Copy link
Collaborator

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?

mbtaylor added a commit to Starlink/starjava that referenced this issue Jun 5, 2024
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.
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

No branches or pull requests

2 participants