propershark / shark

An event publisher for realtime transit information.
3 stars 0 forks source link

Add `lost` property to vehicles #32

Open faultyserver opened 8 years ago

faultyserver commented 8 years ago

In the course of an agency's operation, there may be times where a Vehicle is taken out of commission while it is traveling on a Route. I've had a personal experience where a bus caught fire and was unable to continue it's trip. As one of 2 buses on the 25 minute loop, this caused major delays for the route, but was completely ignored by existing transit applications at the time.

Most applications continued reporting the original scheduled stop times for the route (even though the bus that would make those stops was not in service), while others just showed increasing wait times for the bus.

The idea of the lost property of a Vehicle is an attempt to automatically determine situations like these and inform users of major delays as early as possible (rather than just accumulating wait times).

The implementation of this property will be through a Middleware class that adds it to the Vehicle model. The Middleware will also add a hidden last_moved_at property to preserve state through multiple update cycles. On every cycle where the latitude or longitude of the Vehicle changes, last_moved_at will be updated to Time.now. If last_moved_at exceeds a configurable threshold, then lost will be set to true for the Vehicle.

When the lost property is true, the Middleware will not proxy events for that vehicle up the rest of the stack. The Middleware will, however, send a lost event up the stack so that clients can respond accordingly. This middleware should come closely after the Agency in the stack to avoid any miscellaneous events relating to the Vehicle coming from other Middlewares.

Once the position of a Vehicle changes again, it's lost property will be set to false and normal proxying will resume.

Here's a quick implementation of the Middleware:

class Runaway < Shark::Middleware # Names subject to change
  on_install do
    Shark::Vehicle.attribute :lost, default: false, nilable: false, required: false
    Shark::Vehicle.attribute :last_moved_at, default: ->{ Time.now }, nilable: true, required: false
  end

  on 'vehicles', :update  do |event|
    vehicle = event.args.first

    # If the vehicle moved in this cycle, update `last_moved_at`
    vehicle.last_moved_at = Time.now if vehicle.position_changed?

    # Only fire the `lost` event once after losing the vehicle.
    if vehicle.last_moved_at > configuration.threshold && !vehicle.lost?
      vehicle.lost = true
      fire(Shark::Event.new(
        topic: vehicle.identifier,
        type: :lost,
        args: [vehicle],
        kwargs: {},
        originator: vehicle.identifier
      ))
    else
      vehicle.lost = false
      # Potentially fire a `found` event
    end

    # Only proxy the original event if the vehicle is not currently lost.
    fire(event) unless vehicle.lost?
  end
end
elliottwilliams commented 8 years ago

Keep in mind that vehicles that go back to the depot without deactivating, or that are at a particularly long timepoint will trigger lost.

faultyserver commented 8 years ago

Correct. This is very much an extrapolation, so there are bound to be false positives.

Perhaps having the if vehicle.last_moved_at > configuration.threshold conditional be configurable would help this. Something like this, maybe:

require 'active_support/core_ext/numeric/time'

Runaway.configure do |config|
  config.threshold = 10.minutes

  config.lost_condition = Proc.new do |vehicle|
    vehicle.route ? (Time.now - vehicle.last_moved_at) > config.threshold : false
  end
end

In an effort to avoid over-configuration, I think having the default just be the time condition and defaulting to 10 minutes is sufficient for most cases. If we determine that it is not, and we have the ability to write a better heuristic, we can then add that in the configuration.

elliottwilliams commented 8 years ago

Yeah. I don't think configuration of the condition is necessary, my point is more that consumers of the event model ought to know that "lost" might not mean actually lost.

Another direction to take this: Can the lost condition be based on the vehicle not being stopped at its next_station on a timepoint? This entails calling timetable.next_visit(vehicle.station, vehicle.route) and only setting lost if Time.now is not between the returned ETA and ETD.

The downside to this would be a dependency on Timetable from a relatively low-level Shark middleware.

faultyserver commented 8 years ago

I'm certainly open to suggestions on better lost conditions, but - from the way I'm reading it - I don't think what you described would work well, aside from the fact that it's adding a dependency.

What happens when a vehicle is 11 minutes behind schedule and gets stuck at a stop light? My understanding of your idea says that the vehicle would be lost, since Time.now ⊈ (ETA, ETD).

I do think that the "stationary for 10 minutes" conditional is rather naive, and would like to see it substituted. Am I misunderstanding your suggestion?

elliottwilliams commented 8 years ago

I should have added "additionally" to my explanation. I'm suggesting that the lost condition works like you described, with a n-minutes-without-movement threshold, with the additional condition that the vehicle isn't stopped because it is at a long timepoint. A vehicle shouldn't become lost because it's waiting for an upcoming departure time.

FWIW, I don't think CityBus has anything other than 5 minute timepoints. If you want to check for yourself, this awk script prints every nonequal ETA and ETD: awk -F , '$2 != "" && $2 != $3 { print $1,$2,$3}' stop_times.txt

faultyserver commented 8 years ago

That makes a lot more sense. I'm not sure why I didn't assume you meant additionally.

However, I'm not sold on the benefits of using this heuristic with the cost of having RPCs in the middleware stack. I think there are still plenty of cases where buses would be wrongly considered "lost". The main one that comes to mind is when drivers leave the bus to take mandated breaks: the bus is technically active, and travelling on a route, but can often be delayed by 10+ minutes (I've seen one driver take 16 minutes to return to the bus).

My thoughts for now are to just have the time condition, and see how many lost events get fired in the course of a week or so. I think crafting a better heuristic will be much easier with some experiential data.

elliottwilliams commented 8 years ago

Sure. I'm all for keeping things simple until we actually detect a problem here :)