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

77 weighted points #82

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

77 weighted points #82

wants to merge 7 commits into from

Conversation

Robinlovelace
Copy link
Collaborator

  • Add york_minimal, copied from york example as starting point
  • Visualise basic inputs (hide output files)

@Robinlovelace Robinlovelace linked an issue Sep 24, 2024 that may be closed by this pull request
2 tasks
@Robinlovelace
Copy link
Collaborator Author

Got a nice minimal exmaple going on, with real school locations from OSM:

image

@Robinlovelace
Copy link
Collaborator Author

Weighted points added:

image

@dabreegster OK to add the minimal input files? They are tiny, with 9 subpoints as shown above.

@Robinlovelace Robinlovelace marked this pull request as ready for review September 24, 2024 15:54
@Robinlovelace
Copy link
Collaborator Author

Illustration of the inputs. I plan to push the associated minimal .csv and .geojson files to the folder, unless any objections.

image

Update to make destinations points
@Robinlovelace
Copy link
Collaborator Author

In terms of trying to run od2net on this example, I'm hitting an issue that seems also to apply to the pre-existing examples/york/ example:

docker run -v $(pwd):/app ghcr.io/urban-analytics-technology-platform/od2net:main /app/config.json
Using config from /app/config.json:
{
  "requests": {
    "description": "Manually drawn zones and flows to a few named destination points",
    "pattern": {
      "ZoneToPoint": {
        "zones_path": "zones.geojson",
        "csv_path": "od.csv",
        "destinations_path": "destinations.geojson",
        "origin_zone_centroid_fallback": false
      }
    },
    "origins_path": "buildings.geojson",
    "destinations_path": "destinations.geojson"
  },
  "cost": {
    "ExternalCommand": "python3 cost.py"
  },
  "uptake": "Identity",
  "lts": {
    "ExternalCommand": "python3 lts.py"
  },
  "elevation_geotiff": null
}

## everything
#### Load network
Trying to load network from /app/intermediate/network.bin
That failed (failed to open file `/app/intermediate/network.bin`), so generating it from /app/input/input.osm.pbf
###### Make Network from xml or pbf
######## Scrape OSM data
######## Scrape OSM data took 298.472411ms
  Got 553,457 nodes, 20,799 ways, and 3,016 amenities
######## Split into edges
######## Split into edges took 57.8848ms
  Split into 38,960 edges
######## Building RTree for matching amenities to edges
######## Building RTree for matching amenities to edges took 50.187265ms
######## Match amenities to closest edge
######## Match amenities to closest edge took 4.407652ms
######## Calculate LTS for all edges
thread 'main' panicked at od2net/src/plugins/lts.rs:20:87:
called `Result::unwrap()` on an `Err` value: No such file or directory (os error 2)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
######## Calculate LTS for all edges took 30.92225ms
WARNING: Dropping timer during block Make Network from xml or pbf. Probably crashing.

Copy link
Collaborator

@dabreegster dabreegster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result looks reasonable to me
image

"destinations_path": "destinations.geojson"
},
"uptake": "Identity",
"cost": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be simpler to not use a custom external command for lts and cost in this example. That's causing the Docker issue, and other examples demonstrate how to do this already. How about following the Liverpool example for cost and lts? Do you want something like quiet routes for this, just distance / direct roues, or something else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, simple is good, clearly just copy-pasted from previous York example. Can update that, good suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I want to carve out really clear terminology in this project, and "subpoints" does not seem ideal. Since these are used as origins, what about origins.geojson?
  2. The property needs to be called weight, not size. I need to document this still

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objection to origins.geojson, other than slight name clash with 'zones of origin': you can have zones in which flows originate and origins that specificy the exact points they must flow from. But as long as it's documented could well be the least bad options, I have no better suggestions, and happy with origins.geojson if you are. Subpoints comes from the jittering paper and it seems we're using the word in a similar way, what's the objection to subpoints out of interest?

👍 to clarify terms upfront and happy to adapt.

]
)

def makeZones():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the files are checked into git directly, we don't need these two. Actually, I want to revisit this in the York example, where it's copied from. Not sure why I chose to do it that way...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's a bit of an unusual way of creating a GeoJSON, why not just include the .geojson as a config! OK to wait for the York example to be updated before updating this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to check both qmd and md into git, because the second is derived from the first, and it contains lots of log-spam. Looking at https://quarto.org/docs/publishing/github-pages.html, checking in both is option 1, right? Publishing to GH Pages is more work upfront, but feels like the better option. I can set that up after we get this first PR merged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, happy to remove the .md. Glad I didn't check in the rest of the stuff that quarto outputs based on this comment!

python setup.py
```

We'll get a sample of 2 schools in York (York High School and Huntington School) using the `osmextract` package.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine showing examples like this here, but FWIW, something like Overpass can be quicker/more interactive. Ultimately a user guide could point to a variety of methods for generating inputs like this. https://overpass-turbo.eu/s/1RL9 as an example

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes Overpass is fine for this also. Happy for this to be tweaked, just using what I know best. I don't think it matters here but happy to use another tool to get the schools if there are ways of doing that with reproducible and readable code. Doesn't seem like a biggy or blocker at this stage but let me know if you'd like to see changes to this bit.

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.

Weighted points
2 participants