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

fix(bq,sf,rs,pg|quadbin): improve precision of long lat conversion near the latitude limits #461

Merged
merged 9 commits into from
Jan 11, 2024

Conversation

jgoizueta
Copy link
Contributor

@jgoizueta jgoizueta commented Dec 28, 2023

Description

Shortcut

Experimental improvement of the QUADBIN_FROMLONGLAT lattitude clipping.

The formula that computes the quadbin Y coordinate from the latitude is unstable near the latitude limits (+/- 85.05112877980659). Because of this, we were clipping the latitude with a smaller value (85.05) to avoid errors that would assign an incorrect quadbin value to latitudes near the limit.

It seems better to clip the values of LN((1 + __sinlat) / (1 - __sinlat)) / pi instead of the input latitude.

A clipping of the input is maintained to avoid errors with latitude=90 or incorrectvalues with latitude>90 (same for negative values). Note it's questionable to allow and clip values greater than 90, because BigQuery would not allow creating a geography with them, and in any case they could be interpreted modulo 90 (so that 130 could be interpreted as 40, and not clipped).

Acceptance

SELECT `carto-un`.carto.QUADBIN_TOZXY(`carto-un`.carto.QUADBIN_FROMLONGLAT(0, 85.051128779806, 26))
[{
  "f0_": {
    "z": "26",
    "x": "33554432",
    "y": "2438"
  }
}]

SELECT `carto-un`.carto.QUADBIN_TOZXY(`cartodb-data-engineering-team`.jgoizueta_carto.QUADBIN_FROMLONGLAT(0, 85.051128779806, 26))
[{
  "f0_": {
    "z": "26",
    "x": "33554432",
    "y": "0"
  }
}]

SELECT `carto-un`.carto.QUADBIN_TOZXY(`carto-un`.carto.QUADBIN_FROMLONGLAT(0, 85.051128779806, 20))
[{
  "f0_": {
    "z": "20",
    "x": "524288",
    "y": "38"
  }
}]

SELECT `carto-un`.carto.QUADBIN_TOZXY(`cartodb-data-engineering-team`.jgoizueta_carto.QUADBIN_FROMLONGLAT(0, 85.051128779806, 20))
[{
  "f0_": {
    "z": "20",
    "x": "524288",
    "y": "0"
  }
}]

For negative latitudes:

SELECT `carto-un`.carto.QUADBIN_TOZXY(`carto-un`.carto.QUADBIN_FROMLONGLAT(0, -85.051128779806, 26))
[{
  "f0_": {
    "z": "26",
    "x": "33554432",
    "y": "67106425"
  }
}]

SELECT `carto-un`.carto.QUADBIN_TOZXY(`cartodb-data-engineering-team`.jgoizueta_carto.QUADBIN_FROMLONGLAT(0, -85.051128779806, 26))
[{
  "f0_": {
    "z": "26",
    "x": "33554432",
    "y": "67108863"
  }
}]

SELECT `carto-un`.carto.QUADBIN_TOZXY(`carto-un`.carto.QUADBIN_FROMLONGLAT(0, -85.051128779806, 20))
[{
  "f0_": {
    "z": "20",
    "x": "524288",
    "y": "1048537"
  }
}]

SELECT `carto-un`.carto.QUADBIN_TOZXY(`cartodb-data-engineering-team`.jgoizueta_carto.QUADBIN_FROMLONGLAT(0, -85.051128779806, 20))
[{
  "f0_": {
    "z": "20",
    "x": "524288",
    "y": "1048575"
  }
}]

@vdelacruzb
Copy link
Contributor

Once this is merged: CartoDB/quadbin-py#17
We will have to update the version of the Redshift module.

Copy link

@vdelacruzb vdelacruzb changed the title fix(bq|quadbin): improve precision of long lat conversion near the latitude limits fix(bq,sf,rs,pg|quadbin): improve precision of long lat conversion near the latitude limits Jan 2, 2024
@vdelacruzb vdelacruzb added dedicated_redshift Deploys an instance of the AT into Redshift and removed Do not merge labels Jan 11, 2024
@vdelacruzb vdelacruzb removed the dedicated_redshift Deploys an instance of the AT into Redshift label Jan 11, 2024
@jgoizueta jgoizueta requested a review from a team as a code owner January 11, 2024 14:47
@vdelacruzb vdelacruzb added the dedicated_redshift Deploys an instance of the AT into Redshift label Jan 11, 2024
Copy link
Contributor

Dedicated carto environment deployed in Redshift host redshift-serverless-shared.515242991920.us-east-1.redshift-serverless.amazonaws.com and carto_dev_data database

@vdelacruzb vdelacruzb merged commit b0a6e08 into main Jan 11, 2024
24 checks passed
@vdelacruzb vdelacruzb deleted the fromlonglat-clipping branch January 11, 2024 14:56
Copy link
Contributor

Dedicated environment removed

@vdelacruzb vdelacruzb mentioned this pull request Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dedicated_redshift Deploys an instance of the AT into Redshift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants