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.

Sign up for GitHub

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

Jump to bottom

Crowding data confidence determination #673

Merged
merged 14 commits into from
Aug 28, 2023
Merged

Crowding data confidence determination #673

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
373e511
Add locations feed engine
PaulJKim Aug 15, 2023
e075fba
Add unit test for 'deletion' logic
PaulJKim Aug 15, 2023
67d4d85
Add a few more tests
PaulJKim Aug 15, 2023
abf5778
remmove io puts
PaulJKim Aug 15, 2023
7b85ec7
fix typedef
PaulJKim Aug 16, 2023
8e7d492
Begin reading in multi_carriage_details field
PaulJKim Aug 16, 2023
7e98cfd
Calculate crowding data confidence level
PaulJKim Aug 16, 2023
f6ce48b
Branch arrival and approaching audio logic based on crowding data con…
PaulJKim Aug 16, 2023
db7d146
Add a todo for where to incorporate read loop selection logic
PaulJKim Aug 16, 2023
165b961
Merge branch 'main' into pk/crowding-data-confidence-determination
PaulJKim Aug 17, 2023
1dd530d
Clean up calculation logic
PaulJKim Aug 17, 2023
a5f1f17
adjust mapping logic to match actual schema
PaulJKim Aug 17, 2023
edd033c
Address PR comments
PaulJKim Aug 28, 2023
fe97d8d
Use fully qualified name for CarriageDetails type
PaulJKim Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 42 additions & 17 deletions lib/content/audio/predictions.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,26 +46,51 @@ defmodule Content.Audio.Predictions do
]

predictions.minutes == :arriving ->
[
%TrainIsArriving{
destination: predictions.destination,
trip_id: predictions.trip_id,
platform: predictions.platform,
route_id: predictions.route_id
}
]
if predictions.crowding_data_confidence == :high do
# TODO: Pass along crowding data classification once available
[
%TrainIsArriving{
destination: predictions.destination,
trip_id: predictions.trip_id,
platform: predictions.platform,
route_id: predictions.route_id
}
]
else
[
%TrainIsArriving{
destination: predictions.destination,
trip_id: predictions.trip_id,
platform: predictions.platform,
route_id: predictions.route_id
}
]
end

predictions.minutes == :approaching and (line == :top or multi_source?) and
predictions.route_id in @heavy_rail_routes ->
[
%Approaching{
destination: predictions.destination,
trip_id: predictions.trip_id,
platform: predictions.platform,
route_id: predictions.route_id,
new_cars?: predictions.new_cars?
}
]
if predictions.crowding_data_confidence == :high do
# TODO: Pass along crowding data classification once available
[
%Approaching{
destination: predictions.destination,
trip_id: predictions.trip_id,
platform: predictions.platform,
route_id: predictions.route_id,
new_cars?: predictions.new_cars?
}
]
else
[
%Approaching{
destination: predictions.destination,
trip_id: predictions.trip_id,
platform: predictions.platform,
route_id: predictions.route_id,
new_cars?: predictions.new_cars?
}
]
end
Comment on lines +72 to +93
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking out loud: Not sure how this will end up looking in the following PRs but hopefully there's an opportunity to unify some of this logic a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep! This does get unified down the line


predictions.minutes == :approaching ->
[
Expand Down
28 changes: 25 additions & 3 deletions lib/content/message/predictions.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ defmodule Content.Message.Predictions do
platform: nil,
new_cars?: false,
terminal?: false,
certainty: nil
certainty: nil,
crowding_data_confidence: nil
]

@type t :: %__MODULE__{
Expand All @@ -47,7 +48,8 @@ defmodule Content.Message.Predictions do
zone: String.t() | nil,
platform: Content.platform() | nil,
terminal?: boolean(),
certainty: non_neg_integer() | nil
certainty: non_neg_integer() | nil,
crowding_data_confidence: :high | :low | nil
}

@spec non_terminal(
Expand Down Expand Up @@ -77,6 +79,14 @@ defmodule Content.Message.Predictions do
true -> predicted_time |> Kernel./(60) |> round()
end

crowding_data_confidence =
calculate_crowding_data_confidence(
prediction,
Engine.Locations.for_vehicle(prediction.vehicle_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doing this lookup here means we'll end up grabbing location info for every prediction message, every tick, even ones far away or on other lines. Granted, it's an indexed operation, so it may be fast enough, but it's not needed the vast majority of the time. What about storing the vehicle_id in the Message, and then performing this calculation only when we actually need the result?

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 that's a good point. I did end up thinking about that later down the line in a later PR and changed the logic so that we only fetch location if it's an Orange Line prediction. Take a look at this logic for do_crowding/1 and let me know what you think

Copy link
Collaborator

Choose a reason for hiding this comment

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

That does improve things. And the lookup should be fast enough, so it's probably not worth worrying about.

)

# TODO: Calculate crowding data classification and pass that along as well

case Content.Utilities.destination_for_prediction(
prediction.route_id,
prediction.direction_id,
Expand All @@ -95,7 +105,8 @@ defmodule Content.Message.Predictions do
station_code: station_code,
zone: zone,
platform: platform,
certainty: certainty
certainty: certainty,
crowding_data_confidence: crowding_data_confidence
}

{:error, _} ->
Expand Down Expand Up @@ -143,6 +154,17 @@ defmodule Content.Message.Predictions do
end
end

defp calculate_crowding_data_confidence(_prediction, nil), do: nil

defp calculate_crowding_data_confidence(prediction, location)
when prediction.route_id in ["Orange"] and location.stop_id == prediction.stop_id do
if location.status in [:incoming_at, :in_transit_to],
do: :high,
else: :low
end

defp calculate_crowding_data_confidence(_prediction, _location), do: nil

defimpl Content.Message do
require Logger

Expand Down
32 changes: 29 additions & 3 deletions lib/engine/locations.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,42 @@ defmodule Engine.Locations do
defp location_from_update(location) do
%Locations.Location{
vehicle_id: get_in(location, ["vehicle", "vehicle", "id"]),
status: status_to_atom(location["vehicle"]["current_status"]),
status: vehicle_status_to_atom(location["vehicle"]["current_status"]),
stop_id: location["vehicle"]["stop_id"],
timestamp: location["vehicle"]["timestamp"],
route_id: location["vehicle"]["trip"]["route_id"],
trip_id: location["vehicle"]["trip"]["trip_id"],
consist: location["vehicle"]["vehicle"]["consist"]
consist: location["vehicle"]["vehicle"]["consist"],
multi_carriage_details:
location["vehicle"]["multi_carriage_details"] &&
Enum.map(
location["vehicle"]["multi_carriage_details"],
&parse_carriage_details/1
)
}
end

defp status_to_atom(status) do
defp parse_carriage_details(multi_carriage_details) do
%Locations.CarriageDetails{
label: multi_carriage_details["label"],
occupancy_status: occupancy_status_to_atom(multi_carriage_details["occupancy_status"]),
occupancy_percentage: multi_carriage_details["occupancy_percentage"],
carriage_sequence: multi_carriage_details["carriage_sequence"]
}
end

defp occupancy_status_to_atom(status) do
case status do
"MANY_SEATS_AVAILABLE" -> :many_seats_available
"FEW_SEATS_AVAILABLE" -> :few_seats_available
"STANDING_ROOM_ONLY" -> :standing_room_only
"CRUSHED_STANDING_ROOM_ONLY" -> :crushed_standing_room_only
"FULL" -> :full
_ -> :unknown
end
end

defp vehicle_status_to_atom(status) do
case status do
"INCOMING_AT" -> :incoming_at
"STOPPED_AT" -> :stopped_at
Expand Down
18 changes: 18 additions & 0 deletions lib/locations/carriage_details.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
defmodule Locations.CarriageDetails do
defstruct label: nil,
occupancy_status: nil,
occupancy_percentage: nil,
carriage_sequence: nil

@type t :: %__MODULE__{
label: String.t(),
occupancy_status:
:many_seats_available
| :few_seats_available
| :standing_room_only
| :crushed_standing_room_only
| :full,
occupancy_percentage: non_neg_integer(),
carriage_sequence: non_neg_integer()
}
end
6 changes: 4 additions & 2 deletions lib/locations/location.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ defmodule Locations.Location do
timestamp: nil,
route_id: nil,
trip_id: nil,
consist: []
consist: [],
multi_carriage_details: []

@type t :: %__MODULE__{
vehicle_id: String.t() | nil,
Expand All @@ -14,6 +15,7 @@ defmodule Locations.Location do
timestamp: DateTime.t() | nil,
route_id: String.t() | nil,
trip_id: String.t() | nil,
consist: list()
consist: list(),
multi_carriage_details: list(CarriageDetails.t())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This type needs the fully qualified name now.

}
end
2 changes: 2 additions & 0 deletions lib/signs/utilities/audio.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ defmodule Signs.Utilities.Audio do
{[], sign.announced_approachings, sign.announced_arrivals},
fn audio, {new_audios, new_approaching_trips, new_arriving_trips} ->
case audio do
# TODO: Add a check for whether high-confidence crowding info is present and whether it has already been announced with an approaching message
%Audio.TrainIsArriving{trip_id: trip_id} when not is_nil(trip_id) ->
if audio.trip_id in sign.announced_arrivals do
{new_audios, new_approaching_trips, new_arriving_trips}
Expand All @@ -154,6 +155,7 @@ defmodule Signs.Utilities.Audio do
[audio.trip_id | new_arriving_trips]}
end

# TODO: Start tracking trip_ids where we announce high-confidence crowding info with Approaching
%Audio.Approaching{trip_id: trip_id} when not is_nil(trip_id) ->
if audio.trip_id in sign.announced_approachings do
{new_audios, new_approaching_trips, new_arriving_trips}
Expand Down
Loading