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

Review coordinate limits #99

Merged
merged 3 commits into from
Nov 6, 2023
Merged

Conversation

gmantele
Copy link
Collaborator

As raised in RFC-2.1 (Grid and DAL WGs), coordinates limits given in this document are now obsolete since we deprecated the coordinate system argument in geometrical functions.

In ADQL-2.0 and the current state of ADQL-2.1, coordinates are limited to [0, 360] and [-90, 90]. But if any coordinate system is allowed, these ranges may not be valid anymore. Galactic coordinates are usually limited to [-180, 180] and [-90, 90].

What should we do?

  1. Just remove this section about limits (but still keep the constraint about spherical coordinates being expressed in degree)?
  2. Give an exhaustive list of ranges for each coordinate system? (what I have just tried to do in this PR)
  3. Give a relaxed couple of coordinates limits (e.g. [-180, 360])?

I am not an expect in term of coordinate systems and coordinates limits, so, any advice here is very welcome 🙂

@gmantele gmantele added the bug Something isn't working label Oct 11, 2023
@gmantele gmantele added this to the 2.1 milestone Oct 11, 2023
@msdemlei
Copy link
Contributor

msdemlei commented Oct 11, 2023 via email

@gmantele
Copy link
Collaborator Author

I agree, there is a lot of grey area about coordinates, winding sense and polygons. I also agree on the point that ADQL should not say anything about how to deal about all these things...it is an interoperability question that should be answered in a more general purpose standard (as you said, DALI, Coords, or ...).

@gmantele
Copy link
Collaborator Author

Here is a new proposal. Instead of removing for good this section, I keep it without any explicit coordinate limits. I just replace limits by a reference to the coordinate system in which coordinates are expressed. This way, there is no explicit idea of "constraint".

I did not want to relax this too much: as you said, it would be pretty bad that users start providing weird coordinates (e.g. ra=-10). First, because nobody will know what the ADQL processor will really do and then, because it will be really hard to come back in a future version. At least now, this sentence convey some idea of limits without giving any explicit one. Users may keep being reasonable with their coordinates.

So, here I just tried to anticipate a future version of ADQL in which we will be able to make a reference to a proper standard about coordinate limits.

What do you think about it? Is it a bad idea? Is the wording confusing and should be rephrased another way?

@msdemlei
Copy link
Contributor

msdemlei commented Oct 12, 2023 via email

@pdowler
Copy link

pdowler commented Oct 12, 2023

In DALI the goal in providing limits was to eliminate the need for range reduction of angles, but I think we did inadvertently make geometry less usable (eg for coordinate systems like galactic that have a different range of valid longitude).

So the language/fix in DALI should be more along those lines; it would apply to point, circle, polygon (multipolygon and shape refer back to those existing types so they are ok).

We could fix (erratum) DALI by just clarifying the intent with "In equatorial coordinatesystems, ...".

To make geometry usable in galactic coords, we would need to add "In coordinate systems with different valid ranges, the applicable range of values applies. In all cases, range reduction is never ???" (expected? assumed? required?) but that's something we could add in WD-DALI-1.2... here I am thinking about a TAP service with a table of geometries and the metadata for the column says the values are in galactic, or maybe a

iirc, we decided that it's best for no one to do range reduction of values that are passed over the network (in params or responses) to keep the protocols simple.

@gmantele
Copy link
Collaborator Author

Ok. Can we then make a reference in ADQL 2.1 to DALI (as suggested by Markus) + make an erratum to DALI? In the meantime, Pat can work on improving this in WD-DALI-1.2. Does it sound like the way to go with this issue?

@pdowler
Copy link

pdowler commented Nov 1, 2023

agree - will work on improving DALI asap

@gmantele
Copy link
Collaborator Author

gmantele commented Nov 3, 2023

Here is an update.
What do you think?

Should it be a SHOULD or a MUST in the "Coordinates limits" section?

Should we keep the following paragraph starting with "Note that at the time ..."?

@gmantele
Copy link
Collaborator Author

gmantele commented Nov 3, 2023

In parallel of the DALI-1.2 WD update, should we prepare an Erratum for DALI-1., as you suggested @pdowler ?
If yes, as DALI editor, @pdowler , could you do it?

@pdowler
Copy link

pdowler commented Nov 3, 2023

I think the text you have in there now is good.
I am working on an update for WD-DALI-1.2 and extract the relevant bits for an errata on 1.1

@gmantele
Copy link
Collaborator Author

gmantele commented Nov 5, 2023

Great :-) Thanks @pdowler

@gmantele gmantele merged commit 898410c into ivoa-std:master Nov 6, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants