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

add support for parsing subsections of OSM files based on name, e.g., "New South Wales", or "Sydney", instead of rect bounding box only #56

Open
MarkRaadsen opened this issue Apr 20, 2024 · 9 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Milestone

Comments

@MarkRaadsen
Copy link
Member

add support for parsing subsections of OSM files based on name, e.g., "New South Wales", or "Sydney", instead of rect bounding box only

@MarkRaadsen MarkRaadsen added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 20, 2024
@MarkRaadsen MarkRaadsen added this to the Version 0.5.0 milestone Apr 20, 2024
@MarkRaadsen MarkRaadsen self-assigned this Apr 20, 2024
@MarkRaadsen
Copy link
Member Author

Based on OSM tagging. Propose we go for: boundary and then - in simplest form - go for administrative boundary unless explicitly indicated via settings. Then take first match that is found, see https://wiki.openstreetmap.org/wiki/Key:boundary?uselang=en

MarkRaadsen added a commit that referenced this issue Apr 20, 2024
MarkRaadsen added a commit that referenced this issue Apr 21, 2024
… work as before. Not yet implemented support for name based boundary though. todo, not done
MarkRaadsen added a commit that referenced this issue Apr 25, 2024
…o be able to pre-process and extract boundary from OSM relations. NOT DONE YET

- create polygon from OsmBoundary on networkBaseHandler based on settings and use that instead of the one on settings. Then implement osmRelation handling in pre-processing further
MarkRaadsen added a commit that referenced this issue Apr 25, 2024
…ne --> preprocessing of network now needs to be done twice to make this possible with OSM4J: first implement the two phases (enum created but not yet enforced) stage 1: do nothing by register the name and not create the polygon. In stage 2 extract the ways with outer roles and in complete() create the overall polygon from the line strings --> then put it on the network data there and it can be used in main processing (also to be implemented because it is not yet used then remove the initialise bit from the basehandler I think that is still there but is not needed. then feed this through all the way
@MarkRaadsen
Copy link
Member Author

NOTE: needs a bit more sophistication --> bounding area can be defined separately for zoning and services parsing --> yet currently the code to extract this is scattered through preProcessingHandler and network reader. We do not want to copy this across to the Zoning reader and handlers. SOLUTION: Create an OsmBoundingBoundaryBuilder which will contain the code for the various pre-processing steps now in the networkhandler as well as the functionality to then construct a new OsmBoundary. This code can then be reused in network/zoning/services readers. This is the next thing to do <-- REFACTORING, then testing

MarkRaadsen added a commit that referenced this issue Apr 27, 2024
…ne (instead of using settings). However, this is whole support is also need for zoning reader. See comment on issue on how to avoid duplicate code and make this a more general and contained piece of code. NOT DONE. after refactoring, replace use of setting boundingboundary with the one on the data objects passes around.
MarkRaadsen added a commit that referenced this issue Apr 27, 2024
MarkRaadsen added a commit that referenced this issue Apr 28, 2024
…work and zoning reader) + refactored into boundary manager to avoid duplicate code. Base implementation done. Needs thorough testing though. + check if this works with intermodal (not looked at yet)
@MarkRaadsen
Copy link
Member Author

write tests with various bounding boundaries for network only, pt only, network and pt both and intermodal to see if it is working, also test various configs (with boundary type other than administrative and different admin_level values.

MarkRaadsen added a commit that referenced this issue Apr 29, 2024
MarkRaadsen added a commit that referenced this issue May 4, 2024
…ssing functionality based on unit test. NOT DONE
@MarkRaadsen
Copy link
Member Author

It is likely that using a name for the bounding area is more likely to be used than a rectangular bounding box. As such tracking the spanning square bounding box and flagging warnings based on this spanning bb is no longer a good default way of doing things. So, we discontinue using this method as the default and instead replace it by using the bounding boundary instead. Only if we have no bounding boundary we revert to the spanning bounding box

MarkRaadsen added a commit that referenced this issue May 5, 2024
… bugs/missing functionality based on unit test. NOT DONE
MarkRaadsen added a commit that referenced this issue May 5, 2024
… bugs/missing functionality based on unit test. NOT DONE (See issue comment)
@MarkRaadsen
Copy link
Member Author

Testing reveals inefficient construction of OSM ways + currently we make to many nodes/ways available even if they fall entirely outside the bounding area specified. Debug this to fix up. Another general improvement to all of this would be to do the preprocessing of the bounding area before doing anything with preregistering of OSM nodes, this would allow for a more flexible approach where we only preregister nodes within the bounding area, then register OSM ways that have at least one node in the area (fully), and then register all nodes that are part of those ways. This would simplify checking and would also reduce the number of error message regarding missing nodes

MarkRaadsen added a commit that referenced this issue May 5, 2024
@MarkRaadsen
Copy link
Member Author

MarkRaadsen commented May 5, 2024

Further improve: the new approach as per above can cause an OSMway to have NO nodes within bounding area, but due to another OSMway that is intersecting with it to be partially within the bounding area still have some nodes available even though it should NOT be considered --> so if the first node of a link is not available and another is chosen it should be checked if that node falls within the bounding area, if not then we should discard the OSM way quietly....example OSMway 4269134 for melbourne named bounding box test --> then continue refactoring to reduce checks/logging due to this improved approach --> then do the same for zoning

UPDATE: first part dealt with using secondary pre-registration to avoid this problemand not preregister such OSM ways, see comment below on how to best continue. pre-registration of nodes now sorted it seems

MarkRaadsen added a commit that referenced this issue May 5, 2024
@MarkRaadsen
Copy link
Member Author

Partly implemented above. Likely best to instead keep a register (of ids) of OSMWays that were found eligible during preprocessing. Only when they are found eligible we should bother parsing them in the network main processing --> then ease the logging/warning regarding bounding boxes that are now triggered during node collection etc. This approach should be more elegant --> then apply same approach to zoning

MarkRaadsen added a commit that referenced this issue May 11, 2024
…gible Osmways explicitly now and id some refactoring as well.
@MarkRaadsen
Copy link
Member Author

ease the logging/warning regarding bounding boxes that are now triggered during node collection and when creating link because this should now never be needed anymore and we can remove all that somewhat ugly code

MarkRaadsen added a commit that referenced this issue May 12, 2024
…OT DONE -> continue with correct support for zoning/intermodal in same way as we do preprocessing passes for network, then debug based on logging to see if it is working correctly on Melbourne
MarkRaadsen added a commit that referenced this issue May 12, 2024
… reader now implemented in zoning reader. Tests run but partly fail. NEeds detailed debugging and refinements before completion
MarkRaadsen added a commit that referenced this issue Jun 1, 2024
…ns) + made a start with logging based on whether OSM entities are within bounding area. Basics work for this refactor but tests still need fixing for Melbourne. so first task not yet done
@MarkRaadsen
Copy link
Member Author

MarkRaadsen commented Jun 1, 2024

  • fix up existing tests (except Melbourne) so that they pass
  • fix up Melbourne tests so that they pass + debug to see if the zoning portion of the code holds up
  • Melbourne test now with network + zoning + services to see if the services portion of the code holds up
    new implementation and compare, only after that looks good replace because XML only is not easy to compare
  • add stats on how many OSMways and OSMnodes and OSMrelations have been identified out of total in file like in network
  • add stats on how many OSMways and OSMnodes and OSMrelations have been identified out of total in file like in zoning
  • should remove redundant checks for near bounding polygon logging in zoning as well
  • see if we do not lose some platforms etc., as it is slightly less clear what is pt related. Aim is to identify and register all ways/-nodes/relations in pre-processing so in main processing we can assume all are present without exception (and if not then this is a big error rather than a potential one, so logging can be more to the point)
  • Refactor OSM network data parsing by splitting out to spatially eligible from (pre)Registration like in zoning, this is a better approach. Store this info in a separate data class. Possibly the OSMBoundary one. Also zoning stages can be reduced (six) and pre processing stages should ideally be done in their own preprocessing handler
  • check if this is all working well with PLANitGTFS (found a bug in processing stops + we should support a bounding area so we can refactor the near bounding box stuff as we did for OSM to make it more precise

MarkRaadsen added a commit that referenced this issue Jun 2, 2024
…g new approach to boundary. Test results replaced after verification (not tested with services active)
MarkRaadsen added a commit that referenced this issue Jun 6, 2024
…boundary manager to track boundary nodes (no longer in general pool of tracked nodes). Network converter still needs refactoring to support spatial eligibility based on how it is done in zoning though, but prep work for it has been done.
MarkRaadsen added a commit that referenced this issue Jun 9, 2024
…e preprocessing handler to deal with just identifying the bounding boundary. Used by both zoning and network reader, refactoring of preprocessing (non-boundary related_ in network and zoning since no longer needs to deal with bounding boundary stages, so cleaner. Still needs further testing.
MarkRaadsen added a commit that referenced this issue Jun 9, 2024
…ns eligible spatially when they are also eligible for zoning parsing, i.e., PT infrastructure compatible (excluding routes therefore).
MarkRaadsen added a commit that referenced this issue Jun 10, 2024
MarkRaadsen added a commit that referenced this issue Jun 10, 2024
…nd named (increased performance for intermodal reader), updated tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant