Adjusted python-example to work with pydayntic 2 and fixed problem dataclass to expect a list of options - #180
Open
Sagmedjo wants to merge 83 commits into
Open
Adjusted python-example to work with pydayntic 2 and fixed problem dataclass to expect a list of options#180Sagmedjo wants to merge 83 commits into
Sagmedjo wants to merge 83 commits into
Conversation
Updated the 'objectives' field in the 'Problem' class to accept a list of 'Objective' instead of a list of lists. Removed unnecessary calls to update_forward_refs for various classes.
Removed unnecessary calls to update_forward_refs for Pydantic models.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Python interop type definitions to be compatible with Pydantic 2 and fixes the data structure for the objectives field in the Problem class.
- Removed all deprecated
__pydantic_model__.update_forward_refs()calls for Pydantic 2 compatibility - Fixed the
objectivesfield type from nested list to flat list structure - Changes align with the flattening of objectives that occurred in version 1.24.0
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| examples/python-interop/pragmatic_types.py | Removed Pydantic 1 forward reference updates and corrected objectives field type from Optional[List[List[Objective]]] to Optional[List[Objective]] to match the flattened format |
| examples/python-interop/config_types.py | Removed Pydantic 1 forward reference updates for all dataclass definitions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Introduced `minShifts` property in vehicle type schema to enforce minimum shift usage. - Updated fleet generation logic to accommodate minimum shift requirements. - Implemented a new feature to validate minimum shift usage during route planning. - Enhanced goal context creation to include minimum shift features. - Added tests to ensure correct enforcement of minimum shift constraints and their integration with existing features.
…low for breaks with offsets with departure rescheduling
Implement simple way of changing break time windows for offsets to al…
…cles Add even distribution across vehicles
- Implemented a new feature to enforce a minimum number of activities in a tour. - Introduced `create_min_activity_limit_feature` to create a soft constraint for minimum activities. - Added `MinActivityLimitObjective` to calculate penalties for routes not meeting the minimum activity requirement. - Updated the limits checker to validate against the new minimum tour size constraint. - Enhanced vehicle limits to include `min_tour_size`. - Added tests to ensure correct behavior of the new feature, including penalty calculations and validation checks. - Updated existing tests to accommodate the new minimum tour size functionality.
…idation functions
…bjective and added docs
This feature allows configuring time constraints for when jobs can be served: - earliestFirst: earliest time a vehicle can arrive at its first job - latestLast: latest time a vehicle can depart from its last job Useful for enforcing business hours, compliance requirements, and coordinating with external systems.
…on feature - Removed `target_nearest_distance` from `JobTask` struct and related code. - Updated `Objective` enum to replace `MinimizeNearestDistance` with `MinimizeVehicleDistance`. - Implemented a new feature to minimize vehicle distance penalties based on job assignments to nearest vehicles. - Added tests for the new vehicle distance feature, ensuring correct job assignments and penalty calculations. - Adjusted existing tests to remove references to the removed `target_nearest_distance`.
- Simplified function signatures and reduced line length in job time constraint tests. - Consolidated job time constraint creation in vehicle types for better readability. - Improved assertions in tests for clarity and conciseness. - Removed unnecessary whitespace and comments across various test files. - Ensured consistent formatting in vehicle shift definitions and job creation functions. - Added a new test to verify break alignment to activity boundaries when reserved time intersects with service intervals.
…and enhance asset management
… in weighted-sum-scalar) TerritoryObjective::fitness()/estimate() divided by self.shared.reference while fitness_scale() also returned reference. WeightedSumScalar (goal_reader.rs) computes weights[idx] * o.fitness(s) / o.fitness_scale(), so the division applied twice: (pull+push)/reference^2, near-zero, effectively disabling the objective in exactly the weighted-sum composition it's meant for. Sibling features (vehicle_distance, period_balance, tour_compactness) return raw values from fitness()/estimate() and expose scale only via fitness_scale(); territory now matches that convention. Also corrects the TerritoryBalance::Distance/Duration doc comments, which claimed to balance actual travel distance/duration; job_metric's Distance and Duration arms both call nearest_anchor_prox (proximity to nearest anchor), so the two variants are currently equivalent and not true per-metric travel balancing. Adds a regression test pinning fitness() to the raw PULL+PUSH magnitude (180.0 on the swapped-assignment fixture) and fitness_scale() to reference (10.0) separately, so the two can't silently collapse back into each other.
- add driver_id: None to the three vrp-cli VehicleType literals broken by the new field (only surfaced in whole-workspace compile, not per-crate tests) - factor the complex Arc<dyn Fn(..)> types in territory.rs and period_balance.rs into type aliases to clear clippy::type_complexity under -D warnings
Pre-existing lint in decompose_search.rs surfaced only after the earlier type_complexity errors were fixed (compilation had aborted before reaching it).
The territory E2E solves run the full metaheuristic; under tarpaulin's opt-level-0 instrumentation they exceed the default 60s per-test timeout. These test files are excluded from coverage measurement already.
…construction estimate() recomputed nearest_anchor_prox — a pure function of the static fleet anchors + job compatibility — with an O(actors) skill/time-window scan on every insertion evaluation. At fleet scale (e.g. 364 drivers / 1772 jobs) this made initial-solution construction eat the whole solve budget (0 optimization generations), leaving many jobs unassigned. Precompute once at build: per-job sorted compatible-anchor ranking (serves nearest-anchor, nearest-spare, and the distance/duration balance metric) and per-driver nearest-other-anchor proximity (serves push_marginal). estimate() is now O(1). Benchmark (60 drivers/600 jobs): cheapest-insertion construction 5.96s -> 1.06s; balance/overlap behavior unchanged.
…the balance
New optional flag on the territory objective. Default false: the balance spans
every driver, so a driver left with no jobs is a full deficit (PUSH steers work
to it — 'keep all drivers busy'). When true: idle drivers (load 0) are dropped
from the balance and quotas are re-based over the drivers actually used, so
leaving a driver idle is not an imbalance while the used drivers stay balanced
among themselves ('OK to leave drivers idle / reassign them'). In this mode the
per-insertion push signal is also suppressed so construction may concentrate to
the feasible minimum. Pragmatic JSON: allowIdleDrivers (default false).
…st-spare leak Both PULL paths now bill the excess of a job's weighted power distance over the minimum power distance across its compatible anchors, so cross-boundary reaching is penalized consistently and the accepted fitness no longer forgives it for balance. Adds weighted-boundary, leak-fix, and skill-forced guard tests.
When Objective::Territory has no anchors, the goal reader derives balanced medoid seeds over the customer jobs (compact by proximity, weighted so each power cell captures ~equal production value) and matches drivers to them by start-to-seed proximity via the Hungarian algorithm, feeding set_anchors + set_weights. Explicit anchors keep the caller-supplied behaviour.
…apacity) Extends the seed weight loop with a feasibility term: a power cell whose summed service-time load exceeds the driver shift capacity is shrunk regardless of value (w_i += lr_value*(value_target - value_i) - lr_load*max(0, load_i - capacity)). The goal reader feeds per-job service duration as load and the mean shift length as capacity. capacity=INFINITY recovers pure value balancing.
Derived seeds scrambled real solves: distance k-medoids + value-balancing power weights drove weights to ~70x the seed spacing on real data, collapsing 11/14 seeds and stretching survivors (compactness 4.4) so hulls overlapped; the feasibility load term (one shift's capacity vs a multi-day territory's service load) was the worst offender. Replace seed+weight with a capacitated Lloyd placement: k medoids positioned so the nearest-seed partition is both ~equal production value and compact — no weights needed (they stay 0). On the real 1772-job/14-driver run: compactness 1.00, per-cell value within 1.6x (was 4.4 / 249x with 11 empty seeds). Drops the miscalibrated multi-period load term (Plan 4B); horizon-aware capacity balancing is future work.
…ured metric The seed derivation reused the shared create_kmedoids, whose parallel-reduction tie-breaks are non-deterministic — territories (and a value-balance unit test) differed run-to-run. Replace its init with a deterministic farthest-first (central seed + max-min spread) plus plain Lloyd refinement; same input now always yields the same territories. On the real 1772-job/14-driver run: deterministic, compactness 1.00, per-territory value within 1.75x, no collapse. Also derive seeds on the SAME metric the territory objective balances (production value, service duration, or job count for activities/distance) instead of always production value, so derived territories equalize whatever the caller configured.
With balance omitted, the derived seeds previously still equalized job COUNT per territory (metric fell back to 1.0). Pure efficiency intent means no equalizing at all: a zero metric makes the capacity never bind, degenerating the capacitated placement to plain compact k-medoids. On the real 1772-job run this yields the lowest overlap yet (deep 2.2%) and -24% distance vs balanced, at the price of wildly unequal workloads (no quota, minimize-tours concentrates work).
…-driven outliers Value balance was enforced to the last percent by exiling jobs deep into neighbouring territories, because: - push_marginal priced value x per-driver-constant, so it repelled every extra job from an at-quota driver equally (deep-in-cell jobs as hard as boundary ones) and the value factor dwarfed PULL by ~2 orders of magnitude; - there was no slack around the quota, so the solver traded large spatial overlap for a sub-percent balance gain. Fixes: - Deadband (balance_tolerance, default 0.05 in pragmatic, snake or camelCase wire key): a driver is over quota only above quota*(1+tol) and a deficit only below quota*(1-tol), so small imbalances are free. - Location-aware push_marginal: the metric is normalized by the average job metric (onto PULL's distance scale) and priced by max(0, push_reach - gap), where gap is how much deeper a job sits in this driver's cell than in its next-best one. Boundary jobs carry shedding pressure; jobs deeper than the median gap carry none, so an over-quota driver rebalances by giving up its border jobs, not the ones buried in its territory. Validated on a 1772-job / 14-driver run (240s and 720s budgets): deep spatial overlap 13.7% -> 4.5%, cost -18%, at a looser but bounded (tunable) value balance.
…jective The Objective enum's `rename_all = "kebab-case"` renames variants, not the fields of struct variants, so `allow_idle_drivers` was only matched as snake_case on the wire while the field serializer emits camelCase `allowIdleDrivers`. The camelCase key was therefore silently dropped and the flag always read as its false default. Add the camelCase serde alias (mirroring balance_tolerance) so setting it takes effect.
The default random local-search operators only stumble on compactness-improving moves, so a soft territory objective is never systematically enforced and spatially-misplaced jobs linger in foreign cells. TerritoryRelocate seeds on the most misplaced assigned jobs (those whose nearest neighbours mostly belong to another driver) and relocates each onto that "home" driver's best feasible tour. Acceptance is unchanged (the composite search keeps a move only if the full goal's total_order improves), so it never worsens the accepted solution. It self-activates: GoalContext now records its objective feature names and exposes has_objective(), and the operator is appended to the pool only when the goal contains the `territory` objective. For any problem without territory the operator pool is byte-identical to the stock solver.
Adds a vehicleGroup job field to the pragmatic format so jobs tagged with it activate Task 1's vehicle-group hard constraint, binding them to the same vehicle across shifts. Threaded through the Job struct, job reader, problem properties/detection, goal reader registration, and a new violation code. Also fixes the handful of exhaustive Job struct literals across vrp-pragmatic and vrp-cli that needed the new field.
…constraint Bind a subscription's tagged appointments to one vehicle across its shifts via a new vehicle-group hard constraint, keeping the same technician consistent.
…eference workload Isolated A/B (same binary, operator toggled on/off, 3 runs each, 1772-job / 14-driver production-value-balanced run): @720s ON 4.33% vs OFF 4.47% deep overlap; @300S ON 5.06% vs OFF 4.89% — within seed noise, sign flips between budgets, cost/balance unchanged. The overlap reduction (~13.7% → ~4.4%) comes from the objective's balance deadband + location-aware PUSH marginal, not from this relocation (greedy recreate re-derives the same assignment). Kept because it is harmless and may help other shapes/budgets; documented so it is not mistaken for the overlap lever.
…pty routes A route taken from the registry departs at its shift's `earliest`. The travel delta computed by `calculate_travel_leg` includes the waiting time before the target activity, so `TravelLimitConstraint` charged the whole idle stretch between the shift start and the job's time window against `maxDuration`. The departure advance that would absorb it cannot run at that point: `try_advance_departure_time` bails out on `route.tour.get(1)?`, and it is only invoked from post-processing, the reschedule local operator and notify_failure. Consequence on a full-day shift: a job could only open a new tour when its time window opened early enough to fit the whole span under the limit. On a 00:00-23:59 shift with maxDuration 36000 that excluded every job whose window opens after ~10h into the day, even though the resulting tour is a short depot round trip. Measured on a 21763-job production problem (14 vehicles x 312 shifts): 200 jobs had a completely free, trivially feasible (vehicle, shift) slot they could never be placed into. Reclaiming that stretch is not a relaxation, it is the correction of a book-keeping error: moving the departure forward by the leading wait is always legal on a route without jobs, since no other activity's schedule can break and no break (`TimeSpan::Offset`) can be present. Routes that already carry a job are left untouched. Also advance the departure right after a registry route is opened, so the fresh route's `total_duration` is honest from the first insertion on instead of staying pessimistic until the reschedule operator happens to be drawn. Measured with production flags, stock vs patched, same machine: chunk 2026-08 (1853 jobs) 102 -> 76 unassigned chunk 2027-03 (1886 jobs) 19 -> 12 unassigned whole-period refine (21763) 451 -> 249 unassigned No tour exceeds its duration limit in any of these runs (max span 35974s against a 36000s limit), which confirms the departure advance absorbs the reclaimed time in full.
…ution `MinActivityLimitObjective` was a plain sum of per-route deficits, which makes it blind to exactly the move it is supposed to encourage: shifting a job from one under-sized tour to another lowers one deficit by 1 and raises the other by 1, so the sum does not change and the search has no reason to consolidate. The only rewarded move was emptying a source tour completely. On a 21763-job production problem that was 32 rewarded moves against roughly 10000 legal ones. Its `estimate` returned a constant in both move contexts on top of that, so the layer had no influence on route choice during insertion either — it only ever showed up in the global comparison, where it is outranked by territory and tour compactness and therefore almost never decided anything. Squaring the per-route penalty makes evening out under-sized tours an improvement, and reporting the marginal penalty change from `estimate` lets an under-sized route win over an empty one when both are feasible. Placement is unaffected: `choose_best_result` still prefers any success over a failure, and `minimize-unassigned` still dominates the global comparison, so a job whose only feasible option is a fresh route still gets one. This matters because relaxing the duration limit on empty routes (previous commit) removed an accidental consolidation force — opening a tour used to be expensive, which pushed jobs together for the wrong reason. Nothing replaced it. Whole-year production runs, 21763 jobs, 14 vehicles x 312 shifts: stock 451 unassigned, 120 single-job tours, deficit 3005 duration fix only 166 unassigned, 313 single-job tours, deficit 4756 both fixes 186 unassigned, 101 single-job tours, deficit 3188 both fixes (repeat) 144 unassigned, 88 single-job tours, deficit 3115 The two repeats of the same configuration differ by 42 unassigned, so the placement figures of all three fixed variants are within run-to-run noise of each other, while the tour structure difference is far outside it. Territory integrity is unchanged across all four runs: locations served by more than one technician stay between 0.7% and 1.0%.
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.
Fixes: #153
This pull request primarily cleans up the Python interop type definition files by removing all calls to
__pydantic_model__.update_forward_refs()and fixes the structure of theobjectivesfield in theProblemclass.Codebase cleanup and simplification:
__pydantic_model__.update_forward_refs()fromconfig_types.pyandpragmatic_types.py, to comply with pydantic 2 and aboveType definition update:
objectivesfield in theProblemclass fromOptional[List[List[Objective]]]toOptional[List[Objective]], fixing the data structure for objectives to work with the vrp_cli.solve_pragmatic method.