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

Include information on estimates in table response #5259

Merged
merged 9 commits into from
Dec 11, 2018

Conversation

danpat
Copy link
Member

@danpat danpat commented Nov 2, 2018

Issue

If the fallback_speed option is used, it's useful to know which cells contain estimated information.

This PR adds a new data structure to the table API response that looks like this:

UPDATED: param name to fallback_speed_cells from estimated_cells:

  fallback_speed_cells: [
    [1,0],
    [2,3]
  ]

which are the row,column references to cells that contain estimated data.

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

CHANGELOG.md Outdated
@@ -3,6 +3,7 @@
- Table:
- CHANGED: switch to pre-calculated distances for table responses for large speedup and 10% memory increase. [#5251](https://github.com/Project-OSRM/osrm-backend/pull/5251)
- ADDED: new parameter `fallback_speed` which will fill `null` cells with estimated value [#5257](https://github.com/Project-OSRM/osrm-backend/pull/5257)
- ADDED: if `fallback_speed` is used, a new structure `estimated_cells` will describe which cells contain estimated values [#5259](https://github.com/Project-OSRM/osrm-backend/pull/5259)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the estimated_cells feature get a flag of it's own? In some cases this information may not be needed and would be unused extra bytes.

Copy link
Member

Choose a reason for hiding this comment

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

Let's put that into a different PR!

@ghoshkaj ghoshkaj force-pushed the danpat_estimation_information branch from 4ead990 to bb74294 Compare December 10, 2018 19:19
@ghoshkaj ghoshkaj force-pushed the danpat_estimation_information branch from bb74294 to 925e75e Compare December 10, 2018 20:34
@ghoshkaj
Copy link
Member

I've updated the name to fallback_speed_cells. So the response should now look like:

  fallback_speed_cells: [
    [1,0],
    [2,3]
  ]

Copy link
Contributor

@danpaz danpaz left a comment

Choose a reason for hiding this comment

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

Can you update the OP with the new name fallback_speed_cells? LGTM.

@ghoshkaj ghoshkaj merged commit 06e010b into master Dec 11, 2018
@ghoshkaj ghoshkaj deleted the danpat_estimation_information branch December 11, 2018 17:22
ghoshkaj pushed a commit that referenced this pull request Dec 11, 2018
* Revert "Remove estimated_cells value in the response."

This reverts commit 364e35a.

* Update changelog.

* fix linting

* adjust fallback_speed check

* change [].includes to [].indexOf !== -1 for compatibility with node 4

* change param name

* more cuke tests

* fix formatting
datendelphin added a commit to fossgis-routing-server/osrm-backend that referenced this pull request Nov 19, 2020
  - Changes from 5.20.0
    - Features:
      - ADDED: all waypoints in responses now contain a distance property between the original coordinate and the snapped location. [Project-OSRM#5255](Project-OSRM#5255)
      - ADDED: if `fallback_speed` is used, a new structure `fallback_speed_cells` will describe which cells contain estimated values [Project-OSRM#5259](Project-OSRM#5259)
    - Table:
      - ADDED: new parameter `scale_factor` which will scale the cell `duration` values by this factor. [Project-OSRM#5298](Project-OSRM#5298)
      - FIXED: only trigger `scale_factor` code to scan matrix when necessary. [Project-OSRM#5303](Project-OSRM#5303)
datendelphin added a commit to fossgis-routing-server/osrm-backend that referenced this pull request Nov 19, 2020
  - Changes from 5.20.0
    - Features:
      - ADDED: all waypoints in responses now contain a distance property between the original coordinate and the snapped location. [Project-OSRM#5255](Project-OSRM#5255)
      - ADDED: if `fallback_speed` is used, a new structure `fallback_speed_cells` will describe which cells contain estimated values [Project-OSRM#5259](Project-OSRM#5259)
      - REMOVED: we no longer publish Node 4 or 6 binary modules (they are still buildable from source) [Project-OSRM#5314](Project-OSRM#5314)
    - Table:
      - ADDED: new parameter `scale_factor` which will scale the cell `duration` values by this factor. [Project-OSRM#5298](Project-OSRM#5298)
      - FIXED: only trigger `scale_factor` code to scan matrix when necessary. [Project-OSRM#5303](Project-OSRM#5303)
      - FIXED: fix bug in reverse offset calculation that sometimes lead to negative (and other incorrect) values in distance table results [Project-OSRM#5315](Project-OSRM#5315)
    - Docker:
      - FIXED: use consistent boost version between build and runtime [Project-OSRM#5311](Project-OSRM#5311)
      - FIXED: don't override default permissions on /opt [Project-OSRM#5311](Project-OSRM#5311)
    - Matching:
      - CHANGED: matching will now consider edges marked with is_startpoint=false, allowing matching over ferries and other previously non-matchable edge types. [Project-OSRM#5297](Project-OSRM#5297)
    - Profile:
      - ADDED: Parse `source:maxspeed` and `maxspeed:type` tags to apply maxspeeds and add belgian flanders rural speed limit. [Project-OSRM#5217](Project-OSRM#5217)
      - CHANGED: Refactor maxspeed parsing to use common library. [Project-OSRM#5144](Project-OSRM#5144)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants