Repository navigation
Merge dev into master: send each worker only its shapes (#289) - #291
Merged
Merged
Conversation
gtfs2gps() defined its per-shape worker corefun() as a closure inside its
own frame, so every chunk shipped to a furrr worker carried the entire frame:
the full feed after frequency expansion, the shapes, and everything else.
That meant K copies of the feed in memory, K x feed size through PSOCK
sockets before any work started, and the future globals cap refusing large
feeds altogether ("The total size of the 23 globals exported ... exceeds the
maximum allowed size", #277). The closure also rescanned the full trips and
stop_times tables for every shape.
The worker is now a package-level pair: shape_to_gps() is the old body
moved verbatim with the feed references renamed to a per-shape `slice`, and
convert_shape() wraps it with the progress tick and returns any error as a
condition object. gtfs2gps() splits trips, stop_times, stops and geometry per
shape once and maps convert_shape() over the slices, passing the routes table
and the scalar options as arguments; a shape missing from a split gets a
typed empty table so the existing guards report it as before. Failures are
collected from the returned values: the old `badShapes <<-` collector
assigned inside the worker's copy of the frame and never reached the parent
when parallel = TRUE.
Output is unchanged: identical column by column to the previous build on
all five bundled feeds, sequentially and with 2 workers. With
future.globals.maxSize = 4 MiB the previous build fails on fortaleza
("4.71 MiB exceeds 4.00 MiB"); this one runs at 2 MiB. Peak memory of the
process tree with 4 workers: fortaleza 1272 -> 897 MB, berlin 1153 -> 835 MB.
Known change for invalid feeds only: a trip_id listed under two shapes is
converted once, with the first shape in trips.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bring #289 (send each worker only its shapes) into dev
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
+ Coverage 94.32% 94.94% +0.61%
==========================================
Files 17 17
Lines 687 692 +5
==========================================
+ Hits 648 657 +9
+ Misses 39 35 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges
devintomaster: the per-shape slicing change from #289 / #290.What reaches master
gtfs2gps(parallel = TRUE)sends each worker only the trips, stop times, stops and geometry of the shapes it converts, instead of a closure carrying the whole feed. This fixes the future globals-size failure on large feeds (Exceeding the size in futures::getGlobalsAndPackages #277) and lowers peak memory by about 30% with 4 workers (fortaleza 1272 -> 897 MB).parallel = TRUE.The speed-loop vectorisation (#287) is already on master via #288.
Merge check
masterhas #281 (parallelly, version 2.1-4) thatdevdoes not; the two touch different lines ofR/gtfs2gps.Rand merge cleanly.devtools::test(filter = "gtfs2gps")passed locally on the combined tree (6 tests, 0 failures). The full CI matrix runs on this PR.🤖 Generated with Claude Code