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

Database: add an optional step_direction column to the concatenated_operation_step table #4357

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

rouault
Copy link
Member

@rouault rouault commented Dec 20, 2024

and consequently bump database version structure to 1.5

Building a concatenated operation from its steps is a incredibly complex logic, which requires:

  • figuring out in which direction the step must be actually run
  • set source/target CRS on conversions, that natively lack it, by propagating. This is done by propagating CRS from the neighbouring steps... when this is possible. If you chain directly too many conversions, we'll lack the information at some point. Hopefully for records we have, this is mostly just a theoretical concern (I believe, not totally sure...) All that is to satisfy consistency checks, that check that target_crs[op[n]] == source_crs[op[n+1]]. Although in some cases we do need to access the CRS. Typically for a map projection to know the ellipsoid shape. So the alternative would be to attach the ellipsoid shape as a parameter of a map projection (the good old +ellps parameter of PROJ.4 strings) rather than relying on the one of the base geographic CRS.
  • sometimes insert an implicit Geographic<-->Geocentric conversion step
  • other things I might not even suspect.

This change tries to slightly improve this by addressing the first issue mentioned - figuring out the direction - by adding a step_direction column to the concatenated_operation_step table. Obviously as currently the EPSG dataset doesn't include such information (CC @RogerLott with which I've recently discussed that issue), we must still allow a NULL value to mean unknown. For the few records we maintain under our PROJ authority, we can define it though. At least, this improves a bit the readability of our records.

While writing all this, and recalling code I had to write in past years in the pipeline inference logic that need to synthetize intermediate CRS, I'm strongly thinking to what you mentionned @busstoptaktik about the limitations of the current model... Requiring that operation steps have a CRS is a major practical annoyance.

@rouault rouault added this to the 9.6.0 milestone Dec 20, 2024
@rouault rouault added the funded through GSP Work funded through the GDAL Sponsorship Program label Dec 20, 2024
@rouault rouault force-pushed the concatenated_operation_step_direction branch 2 times, most recently from 38b85ed to 6c9ca65 Compare December 20, 2024 01:15
…peration_step table

and consequently bump database version structure to 1.5

Building a concatenated operation from its steps is a incredibly complex
logic, which requires:
- figuring out in which direction the step must be actually run
- set source/target CRS on conversions, that natively lack it, by
  propagating. This is done by propagating CRS from the neighbouring
  steps... when this is possible. If you chain directly too many conversions,
  we'll lack the information at some point. Hopefully for records we
  have, this is mostly just a theoretical concern (I believe, not
  totally sure...)
  All that is to satisfy consistency checks, that check that
  target_crs[op[n]] == source_crs[op[n+1]]. Although in some cases we do
  need to access the CRS. Typically for a map projection to know the
  ellipsoid shape. So the alternative would be to attach the ellipsoid
  shape as a parameter of a map projection (the good old +ellps
  parameter of PROJ.4 strings) rather than relying on the one of the base
  geographic CRS.
- sometimes insert an implicit Geographic<-->Geocentric conversion step
- other things I might not even suspect.

This change tries to slightly improve this by addressing the first issue
mentioned - figuring out the direction - by adding a step_direction column to the
concatenated_operation_step table. Obviously as currently the EPSG
dataset doesn't include such information (CC @RogerLott with which I've
recently discussed that issue), we must still allow a NULL value to mean
unknown. For the few records we maintain under our PROJ authority, we can
define it though. At least, this improves a bit the readability of our records.

While writing all this, and recalling code I had to write in past years
in the pipeline inference logic that need to synthetize intermediate CRS,
I'm strongly thinking to what you mentionned @busstoptaktik about the
limitations of the current model... Requiring that operation steps have
a CRS is a major practical annoyance.
@rouault rouault force-pushed the concatenated_operation_step_direction branch from 6c9ca65 to 1078177 Compare December 20, 2024 01:42
@RogerLott
Copy link

I will make the IOGP Geodesy Subcommittee aware. My personal opinion (not necessarily shared by the subcommittee) is that this assists implementation and therefore is a good idea, but (a trivial detail) if implemented in EPSG would likely use value of 'reverse' rather than 'inverse' as this is the term used in method formulas in Guidance Note 7-2. I will also take it to the ongoing 19111 revision deliberations, but in that abstract spec this might consider the issue an implementation detail so out of scope.

@jjimenezshaw
Copy link
Contributor

I will make the IOGP Geodesy Subcommittee aware. My personal opinion (not necessarily shared by the subcommittee) is that this assists implementation and therefore is a good idea, but (a trivial detail) if implemented in EPSG would likely use value of 'reverse' rather than 'inverse' as this is the term used in method formulas in Guidance Note 7-2. I will also take it to the ongoing 19111 revision deliberations, but in that abstract spec this might consider the issue an implementation detail so out of scope.

Thanks @RogerLott . It would be great having it in the next big release.

IMHO it is not only an implementation detail. Recently I had to define a concatenated operation, and I spent some time looking for how to specify the direction. I could not expect that there was no such thing. It seemed to be not completely defined.

@rouault
Copy link
Member Author

rouault commented Dec 22, 2024

would likely use value of 'reverse' rather than 'inverse'

ok, I've changed this PR to 'reverse'

IMHO it is not only an implementation detail.

Deeply second that. I can't possibly be the only one to have suffered from that, or nobody has seriously tried to implement 19111 concatenated operations to transform coordinates.

@rouault rouault merged commit 4bacf90 into OSGeo:master Jan 2, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
funded through GSP Work funded through the GDAL Sponsorship Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants