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

Comparison with momepy #2

Open
fnattino opened this issue Oct 28, 2024 · 2 comments
Open

Comparison with momepy #2

fnattino opened this issue Oct 28, 2024 · 2 comments
Assignees

Comments

@fnattino
Copy link
Contributor

Detailed comparisons with the momepy implementation for a real case: right now I have tested this work with a toy example and I get reasonable results for a real network, but we need to make sure we get exact same output (or understand why we don't).

@fnattino
Copy link
Contributor Author

fnattino commented Oct 30, 2024

After #13 , I have "manually" compared results from rcoins with the ones from momepy.COINS for a real case (Bucharest street network) and I found:

  • the same number of strokes are produced by the two codes;
  • results are (topologically) equal: for all (A, B) geometry pairs, we have "(A is within B) AND (B is within A)" (see e.g. description here).

So I am pretty confident the results of rcoins are phisically sound - so feel free to give rcoins a try @cforgaci !

If directly compared for equality, stroke geometries computed with the two packages are not found to be equal because the two code bases can produce linestrings with different point orders (e.g. points are ordered in opposite directions, or a different starting point is used in the case of closed rings) as well as different geometry types (when rings/loops are present, momepy.COINS seems to return MultiLinestrings).

@fnattino
Copy link
Contributor Author

fnattino commented Oct 30, 2024

Still, it would be nice to include the comparison between the two codes as a test, to be run on as part of the CI (maybe not at each push but e.g. only when PRs towards mains are opened).

Due to the different nature of the test, which also requires setting up the python environment to run momepy, maybe this should be outside the standard testing framework?

For this we would need to:

  • add some data to the package, ideally following the same way this is done in CRiSP. The script to retrieve data to the package could also similarly fetch the Bucharest street network from OSM.
  • Implement a test that follows the following steps:
    1. run rcoins::stroke on the packaged data (save results to disk?).
    2. make the packaged data (saved as serialized R objects in .rda format) available for use to Python, maybe via loading data in R and saving a temporary gpkg file?
    3. run momepy.COINS on the same dataset, using something like:
    import geopandas
    import momepy
    edges = geopandas.read_file(...)
    continuity = momepy.COINS(edges)
    strokes = continuity.stroke_gdf()
    # save strokes to disk?
    1. load the output of rcoins and momepy, compare for equality as described in Comparison with momepy #2 (comment) - this could be done in either R (with SF) or in Python (with geopandas).
  • Having the test above run as a GitHub action (for every PR to main?).

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

No branches or pull requests

2 participants