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

Take geography into account for address computation #617

Closed
wants to merge 1 commit into from

Conversation

lonvia
Copy link
Collaborator

@lonvia lonvia commented Dec 9, 2021

Use the more complex way of computing the address from Nominatim for computing the rank 30 address. That makes sure that address parts are not blindly taken from the parent but geographic relations are taken into account. When streets go over boundaries, the addresses along that street will be corrected as necessary according to the administrative entity that contains them. See also Nominatim PR osm-search/Nominatim#2082

Requires advanced SQL which is not supported by H2. Thus use the previous simpler query for tests instead.

Sadly, this PR is incompatible with #547. If it gets merged, then import time will again increase by around 50%. So it would be useful if others tested this a bit to figure out if it is worth the performance loss. I haven't made up my mind yet if I want to merge it or not.

See also discussion in #609.

Use the more complex way of computing the address from Nominatim
for computing the rank 30 address. That makes sure that address
parts are not blindly taken from the parent but geographic
relations are taken into account. When streets go over boundaries,
all address fall then into the right side of the boundary.

Requires advanced SQL which is not supported by H2. Thus use the
previous simpler query for tests instead.
@rocainunwired
Copy link

I pulled this branch, compiled with -DskipTests=true as the tests were failing.

Loaded the Nominatim DB with data only from Hyderbad, India.

Total no of records imported by photon: 18983
Import time with master branch: 40 secs (475 records/sec)
Import time with this branch: 43 secs (442 records/sec)

The address hierarchy issue appears to be fixed with this branch on testing a few data points.

@lonvia
Copy link
Collaborator Author

lonvia commented Nov 4, 2024

This isn't really working as intended. Trying a different approach.

@lonvia lonvia closed this Nov 4, 2024
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

Successfully merging this pull request may close these issues.

2 participants