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

Geography-Driven Events #503

Merged
merged 22 commits into from
Dec 8, 2020

Conversation

quicklywilliam
Copy link
Contributor

No description provided.

@quicklywilliam quicklywilliam requested a review from a team May 21, 2020 17:00
@quicklywilliam quicklywilliam requested a review from a team as a code owner May 21, 2020 17:00
@quicklywilliam quicklywilliam requested a review from a team May 21, 2020 17:00
@CLAassistant
Copy link

CLAassistant commented May 21, 2020

CLA assistant check
All committers have signed the CLA.

@quicklywilliam
Copy link
Contributor Author

This is an initial attempt at implementing #480. Note that because #271 has not landed yet I had to be a bit interpretive in how to work things into the current event structure. Happy to clean things up as that issue moves to implementation.

agency/README.md Outdated Show resolved Hide resolved
provider/README.md Outdated Show resolved Hide resolved
@thekaveman thekaveman changed the base branch from master to dev May 21, 2020 23:23
@jfh01 jfh01 added this to the Future milestone Jun 9, 2020
@jfh01
Copy link
Contributor

jfh01 commented Jul 2, 2020

How do folks feel about trying to get this in for 1.1.0? @quicklywilliam @schnuerle @billdirks

@schnuerle schnuerle modified the milestones: Future, 1.1.0 Jul 2, 2020
@schnuerle
Copy link
Member

Feel good about it for 1.1. Milestone marked.

@schnuerle schnuerle added Agency Specific to the Agency API Policy Specific to the Policy API privacy Implications around privacy for the attention of the OMF Privacy Committee Provider Specific to the Provider API labels Jul 8, 2020
@marie-x
Copy link
Collaborator

marie-x commented Sep 15, 2020

Probably want to spell out some use cases for trip_enter_geography and trip_leave_geography. E.g. if you have, say, council districts, would transitioning from one to another generate two simultaneous events (leave, enter)? Would leaving the municipal boundary potentially generate two events (trip_leave, trip_leave_geography)?

Also off the top of my head I'm not coming up with a use case for Policy enforcement where the enter/leave geography events are needed. Do you have some examples of a Policy whose enforcement requires these events once you drop the telemetry requirement?

@quicklywilliam
Copy link
Contributor Author

@Karcass these are interesting questions. I'm wondering if there might be a way that we could unify the events, so that there's just a single trip_leave event issued (with both the jurisdiction and policy geos in event_geographies) instead of a trip_leave_jurisdiction and a trip_leave_geography.

As for uses cases, one that has came up when we started talking to cities about this was for assessing fees. For example, suppose a city wants to assess a 25 cent fee for trips entering or starting downtown during rush hour.

Copy link
Collaborator

@marie-x marie-x left a comment

Choose a reason for hiding this comment

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

Added some suggestions about not conflating telemetry (movement of vehicle in same state) with events (change of vehicle state).

Also we will need to be super clear on how Geographies are communicated, such that the Providers' notions of currently relevant Geographies is in sync with the regulating Agency. Geographies themselves do not have start-dates or end-dates; applicability is for Policies or Jurisdictions or Metrics to specify. Solving that problem cleanly blocks this proposal IMHO.

agency/README.md Outdated Show resolved Hide resolved
general-information.md Outdated Show resolved Hide resolved
provider/README.md Outdated Show resolved Hide resolved
@schnuerle schnuerle added the Geography Items related to the Geography API label Sep 30, 2020
@schnuerle
Copy link
Member

We reviewed this PR as part of the second OMF Working Group Steering Committee release Checkpoint. Both WGSCs had some feedback and I'm documenting it here for discussion.

Note: we definitely want to flag this as a 'beta' feature in the release, so leaving a reminder here.

We were trying to think how geographies can best be aligned to this, how do you know which geography is used for an event?

And how can this idea be addressed in the simplest way initially, with minimal changes or dependencies on other things like Jurisdictions.

Idea 1) Maybe just use all geographies and return data for all of them by default, then the city can decide which geographies are of value. As part of it being beta, recommend cities talking to providers first to provide a list of geographies it applies to.

Idea 2) There could be a new rule type in policy. One that is 'event' and you would define the geography and details there. This may be a bit duplicative, since you would also use that geography in another rule type.

Idea 3) Events could be an optional flag in policy that says that this policy needs to be treated as an event.

What do you all think of these ideas? I like Idea 3, but I'm not sure it meets all of your use cases @quicklywilliam.

@schnuerle schnuerle linked an issue Oct 21, 2020 that may be closed by this pull request
@quicklywilliam
Copy link
Contributor Author

All set! Have also updated the agenda with details of what needs to be covered.

@schnuerle
Copy link
Member

From the Working Group call last week, here is a summary of the discussions and action items that should be done in the next week or two to make this release:

https://github.com/openmobilityfoundation/mobility-data-specification/wiki/Web-conference-notes,-2020.10.22-(Joint-Working-Group-Provider-Services)#minutes

  • Rename the proposal to ‘Geography Driven Events’ (GDE) to reflect that for now the actions are triggered when vehicles enter or leave a geometry, and geometry IDs are returned.
  • If you use GDEs, then cities are required to make policy and geography public and communicate the URLs to provide public transparency in the policy and geographic areas, and reduce authentication burden on providers. Add that language.
  • Ensure this is structured to be a non-breaking change for 1.1.0, and so it can be tested with minimal disruptions.
  • Agreed that 2 fields (enter and leave) are better than one (cross), so leave as is.
  • Added note that new policies can’t be expected to be implemented until 20 minutes after publishing to prevent frequent or real time changes. 20 min delay is good for some more emergency policy situations, but most policy will be communicated days or weeks ahead of time. Ensure this is clear.
  • Add the beta label and link in the appropriate place to allow cities and providers to try it out and give feedback.
  • Beta designation will also also providers to see what their technical burden is to implement and run in production
  • Language needed to clarify that policy driven events only apply when you opt into this and discuss with provider and city.
  • Can be run in parallel with all existing MDS if desired by cities. So you can turn on GDEs and other telemetry, or turn on GDEs and turn off telemetry, or just use telemetry as it is in 1.0.0. Ensure this is clear.
  • Could be issues with GPS in precision float and boundary riding, returning too many events. May need smoothing of data. Could be worth adding some language to spec to account for this.

@schnuerle
Copy link
Member

There's about a week left to get these changes into the PR for this release @quicklywilliam. If you can do that we can save some time on the last WG call Nov 12, though that day is filling up with some other items now like Metrics and k-values, so Nov 5 may be better (though it's of course much sooner!).

@quicklywilliam quicklywilliam changed the title Policy driven events Geography-Driven Events Nov 5, 2020
@quicklywilliam
Copy link
Contributor Author

@schnuerle I'll post an update on this shortly so that we can discuss and resolve most, if not all, of the remaining points.

…ography event

The inclusion of the leave event introduced complexity and ambiguity around which event_geographies should be communicated. For every other event, event_geographies simply consists of every Geography that contains the location of the event. However, for a leave event it is unclear whether event_geographies should contain the geographies the vehicle just left. Removing this event resolves this ambiguity, the list of geographies left can be inferred if neeeded.
@quicklywilliam
Copy link
Contributor Author

quicklywilliam commented Nov 5, 2020

We've now resolved the remaining issues discussed at the WG:

  • mark as beta
  • clarify which geometries are communicated in in the case of an enter/leave. event_geographies
  • add language clarifying that event_location is required if geometries not present
  • add language around events being only required for beta users
  • change title to "geography driven events"
  • add some overall language somewhere describing intent/purpose of "geography driven events"

This change specify that a crossed_geography_boundary event is delivered after the boundary cross has occured – ie the vehicle is now in the new geographies
@quicklywilliam
Copy link
Contributor Author

quicklywilliam commented Nov 6, 2020

One thing I want to call attention to is that in resolving the issues above I decided to eliminate the trip_leave_geography event in favor of a single crossed_geography_boundary. This was something we discussed at the last work group. We saw benefits to both approaches, and no strong preference emerged.

However, I came to a new conclusion in considering how to best clarify the question of which geometries are communicated with these events. The inclusion of the leave event was adding hidden complexity here. For every event type besides leave, the location given is of the vehicle in its new state – not the one it has just transitioned from. However, for a leave event it is unclear – it would seem that event_geographies should contain the geographies the vehicle just left. Removing this event resolves this ambiguity and makes the behavior of event_geographies consistent. If needed, the list of geographies left can be inferred.

As an added bonus, this change allows for greater flexibility in implementations around when a crossed_geography_boundary event occurs. This is likely to be of use during the beta period as we seek to make stronger recommendations to account for GPS inaccuracy, drift, etc.

@marie-x
Copy link
Collaborator

marie-x commented Nov 9, 2020

Name seems a little wordy. :)
How about changed_geographies?

@schnuerle
Copy link
Member

schnuerle commented Nov 11, 2020

William this looks great, thanks for incorporating all the feedback!

Will you be available to present on your changes at tomorrow's WG meeting? No need to do an overview of the whole GDE proposal, but just what's been updated, and the decision to merge the leave/enter geographies.

Also I noticed a few things in the PR that could be update before tomorrow. If you give me edit access (#2 here) I can make them, otherwise here they are:

  • 1. 'geometry' driven events is written in a number of places. Surely just a typo, one I've made before too!
  • 2. can you add DS Store back to the .gitignore for the repo?
  • 3. a new request: could you add an example of GDEs in the /examples folder readme?

policy/README.md Outdated Show resolved Hide resolved
policy/README.md Outdated Show resolved Hide resolved
Copy link
Member

@schnuerle schnuerle left a comment

Choose a reason for hiding this comment

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

Looks good thanks for the changes based on community and city feedback. Will make other minor changes during release candidate preparation (eg like typos with 'telemtry' spelling).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agency Specific to the Agency API Geography Items related to the Geography API Policy Specific to the Policy API privacy Implications around privacy for the attention of the OMF Privacy Committee Provider Specific to the Provider API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Policy-Driven Trip Events
6 participants