distance_influence should not get a default value if no value is specified #2716

karussell · 2023-01-06T16:41:43Z

And the default value of 70 makes this impossible, so remove this.

Users of client-hc now have to set the distanceInfluence to 70 if a lower value is defined on the server-side and can remove it if they want to match the server-side value which is highly recommended for performance reasons.

…ified and the default value of 70 makes this impossible

easbar · 2023-01-06T19:50:35Z

web-api/src/main/java/com/graphhopper/util/CustomModel.java

+    /**
+     * @return the distance influence of this CustomModel. 0 is returned if it wasn't set (i.e. it was null)
+     * @see #hasDistanceInfluence()
+     */


Can we not just return null? null means it's not there, so exactly what we want?

I implemented it this way initially, but this would not be backward compatible (and might throw NPEs in rare situations) and as we and other users would do this "if null then 0 else value" code anyway I thought it might be a good compromise to do this in the getter. But no strong opinion.

Did you commit this somewhere? Where was there a problem? I would prefer to keep it as null in case it is missing. The special value 0 might even be needed for some cases.

Ok, will revert it to this but mention it in the changelog.

easbar · 2023-01-06T19:52:16Z

Is/was this a client-hc-only problem or was distance_influence set to 70 also on the server side when we deserialized, e.g. an empty model?

karussell · 2023-01-06T20:03:12Z

Is/was this a client-hc-only problem or was distance_influence set to 70 also on the server side when we deserialized, e.g. an empty model?

Yes, indeed, this is also a problem.

easbar · 2023-01-09T09:04:02Z

CHANGELOG.md

+- users of client-hc now have to set the distanceInfluence to 70 if a lower value is defined on the server-side and can remove it if they want to match the server-side value which is highly recommended for performance reasons, see #2716
 - there is a new, required 'import.osm.ignored_highways' configuration option that must be used to not increase the graph size and decrease performance for motorized-only routing compared to previous versions, #2702


Can we not put it the other way around: Normally it should be null (so the server-side value will be taken) and users don't need to do anything. And if they set it to some value, well then this value will be used.

I phrased my comment in a way to know what to do when people use an old version and plan to upgrade. But probably this is misleading. What I meant is that after they upgraded to the new version it is recommended to remove the distanceInfluence for performance reasons and if they had a lower value than 70 on the server-side then (after the upgrade) they have to change it to 70 to get the identical routes as before.

Ah I see :) Maybe this:

There is no longer a default value for the distanceInfluence Parameter in custom models sent via client-hc. Previously it was 70. Not setting it explicitly now means the server-side value will be used.

core/src/test/java/com/graphhopper/routing/weighting/custom/CustomWeightingTest.java

web-api/src/main/java/com/graphhopper/util/CustomModel.java

easbar · 2023-01-09T20:51:35Z

web-api/src/main/java/com/graphhopper/util/CustomModel.java

@@ -28,8 +28,7 @@ public class CustomModel {

    public static final String KEY = "custom_model";

-    // e.g. 70 means that the time costs are 25€/hour and for the distance 0.5€/km (for trucks this is usually larger)
-    static double DEFAULT_DISTANCE_INFLUENCE = 70;


Where is the server-side default now? Is it still 70?

No. The server-side default is 0 now.

It has been bugging me for a while now that our "fastest" routing doesn't really pick the fastest route by default.

Maybe mention this in the changelog as well: Server-side profiles with custom weighting now use distance_influence: 0 by default (previously it was 70).

It has been bugging me for a while now that our "fastest" routing doesn't really pick the fastest route by default.

Did it not? For weighting: fastest we (still) get the FastestWeighting, which (almost) returns the fastest route, or do you mean the destination/private penalty? Also for weighting: fastest we use the priority weighting currently for bike for example. But this isn't related to this PR.

As someone who expected the fastest route when using a CustomModel, the default value for "distance_influence" caught me a bit off guard. It is not obvious at the first moment that here the length of the route is included in the weighting by default.

I think that this is not clear to most who use this feature.

Ok, so far I wasn't aware that the expectation of an empty CustomModel is that it yields the fastest route. But anyway, I agree that 0 is probably the better default since 70 is quite arbitrary.

easbar · 2023-01-10T08:41:01Z

core/src/main/java/com/graphhopper/routing/weighting/custom/CustomModelParser.java

@@ -115,7 +115,7 @@ static CustomWeighting.Parameters createWeightingParameters(CustomModel customMo
            CustomWeightingHelper prio = (CustomWeightingHelper) clazz.getDeclaredConstructor().newInstance();
            prio.init(lookup, avgSpeedEnc, priorityEnc, customModel.getAreas());
            return new CustomWeighting.Parameters(prio::getSpeed, prio::getPriority, prio.getMaxSpeed(), prio.getMaxPriority(),
-                    customModel.getDistanceInfluence(), customModel.getHeadingPenalty());
+                    customModel.hasDistanceInfluence() ? customModel.getDistanceInfluence() : 0, customModel.getHeadingPenalty());


Maybe extract a constant as this 0 is the server-side default value and it appears also in FindMinMax.

easbar · 2023-01-10T08:42:09Z

core/src/main/java/com/graphhopper/routing/weighting/custom/FindMinMax.java

+        double qmDI = queryModel.hasDistanceInfluence() ? queryModel.getDistanceInfluence() : 0;
+        double bmDI = baseModel.hasDistanceInfluence() ? baseModel.getDistanceInfluence() : 0;


Here the 0 is also the server-side default? Shouldn't there really only be one place where it appears: The code where the yaml definition of the model is translated to a Java object?

Here the 0 is also the server-side default?

Not sure. Isn't it more an artifact of the "Double" type? It would be implicit for "double".

The code where the yaml definition of the model is translated to a Java object?

Even if your request is without (de)serialization you can still have null values.

Yes, I thought after reading the config file the distance influence field of the server-side model should either be 0 (if nothing was specified) or whatever was specified in the server config, but never null. And when we merge a request custom model with the one on the server-side we either use the one from the server (in case the requesting distance influence is null ) or use the value from the request (in case it was not null).

So basically I think that baseModel.getDistanceInfluence() == null would be a programming error and can be treated as such just let the NullPointerException be thrown.

And for qmDI we don't need a default value, because when nothing is given we will use the server-side value. Written as code:

if (queryModel.getDistanceInfluence() != null && queryModel.getDistanceInfluence() < baseModel.getDistanceInfluence()) throw new IllegalArgumentException("CustomModel in query can only use " + "distance_influence bigger or equal to " + baseModel.getDistanceInfluence() + ", but was: " + queryModel.getDistanceInfluence());

Ok, I see this is not the way we do it currently. We do not apply the default server-side value during the deserialization of config.yml, but only much later in createWeightingParameters. This way it is also applied when we use the Java API. And because we call FindMinMax even before that we need to pay attention that there is no value yet :( Maybe it would be cleaner to use two separate classes for the server-side/profile CustomModel and the one used for the request.

otbutz · 2023-01-10T08:56:14Z

If the default value of the server is 0, should the restriction on the client side also be removed?

karussell · 2023-01-10T08:59:29Z

Do you mean for our API? There the distance_influence value of the server is not 0. But we should probably not throw an exception and e.g. include a warning in the response as it makes increasing the distance influence on the server-side very tricky.

easbar · 2023-01-10T09:20:34Z

Why would a warning in an (otherwise empty?) response be better than an error code? Increasing the distance influence on the server-side will make requests with distance influence values higher than the old but lower than the new value impossible, yes. But we could also fall back to A* in this case.

karussell · 2023-01-10T10:59:50Z

Why would a warning in an (otherwise empty?) response be better than an error code?

It will not break existing client requests. (I meant a normal routing response with a warning stating that the distance influence was ignored.)

…ified (graphhopper#2716) * distance_influence should only be sent to server when explicitly specified and the default value of 70 makes this impossible * fix changelog * changelog * CustomModel.getDistanceInfluence: allow null values * remove comment

@ihsanguldur

* force max_slope and average_slope to create SlopeCalculator * remove that motorroad=yes influences speed or is used as highway tag, graphhopper#2329 * CurvatureCalculator: if included in tests that don't set artificial tags * Treat segments tagged as "lcn=yes" the same way as a bicycle route relation with "network=lcn". (graphhopper#2693) See https://wiki.openstreetmap.org/wiki/DE:Tag:lcn%3Dyes * update maps * Fix rare NPE in map matching * Add support for (single) via-way restrictions (graphhopper#2689) * Remove outdated comment and rule * Revert "Remove outdated comment and rule" This reverts commit 03e64da. * value can be no for oneway:bicycle but also for cycleway:right:oneway, fixes graphhopper#2694 * Replace com.wdtinc MVT library with no.ecc (graphhopper#2698) * Upgrade to JTS 1.19.0, fix graphhopper#1803 * minor * Support ";" access delimiter for bikes (graphhopper#2676) One example is vehicle = agricultural;forestry Now we block also such combinations for bikes. * Update maps, geocoding is disabled now * Remove LegacyProfileResolver (graphhopper#2679) * removed legacy parameters * fixed error message * add comment * Update README.md * Copy no.ecc code, remove no.ecc repository, graphhopper#2698 * Create edges even if flags are empty (graphhopper#2700) * Add OSM way ID encoded value (graphhopper#2701) * remove Bike2WeightTagParser again (graphhopper#2668) * fix changelog graphhopper#2703 * Use explicit ignore list for highway values during OSM import (graphhopper#2702) * Revert "Use explicit ignore list for highway values during OSM import (graphhopper#2702)" This reverts commit ef2937f. * split buildEncodingManagerAndOSMParsers method * Revert "Revert "Use explicit ignore list for highway values during OSM import (graphhopper#2702)"" This reverts commit 97579b6 * Revert "Revert "Revert "Use explicit ignore list for highway values during OSM import (graphhopper#2702)""" This reverts commit daccf5f. * Revert "Revert "Revert "Revert "Use explicit ignore list for highway values during OSM import (graphhopper#2702)"""" This reverts commit b9b1ed4. * Allow ignoring vehicle tag parser name * Add tests to demonstrate that moped=yes etc. does not allow driving on a footway * Keep ways tagged as man_made=pier or platform=railway for foot/bike * Footway encoded value (graphhopper#2707) * Add ignored highways to benchmark.sh * do not throw an error if Java config is used without config file * Revert "do not throw an error if Java config is used without config file" This reverts commit c872780. * Revert ignored highways for now * Use GitHub Actions for Maven Central deployment too (graphhopper#2709) * use github actions to deploy to maven central * use private key directly to avoid escaping problems * run only when tagged; renamed * specify key id * use more recent plugin versions * different badge * github actions: ignore tags * include branches required * include (2nd) test build to be sure * ignoring tags is the default if branches is specified * matches really all tags, https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet * force test failure * Revert "force test failure" This reverts commit 85b38fe. * include bus into TransportationMode enum, graphhopper#2710 * log the exception, not just the message * mapmatching: put visited nodes into log event * github packages: set permissions * open a few things up * vehicle -> profile * remove unused non-private abstract method * Implement edge-to-edge routing for unidir AStar * Revert "Revert ignored highways for now" This reverts commit 03978b8. * Fix test * Put back test (?) * Rename publish-github-actions.yml * Enable LM for small benchmark map and A* measurements for small map * Remove name, turnCostEnc and getAccess from VehicleTagParser (graphhopper#2713) * make CustomWeighting.Parameters public, graphhopper#2717 * distance_influence should not get a default value if no value is specified (graphhopper#2716) * distance_influence should only be sent to server when explicitly specified and the default value of 70 makes this impossible * fix changelog * changelog * CustomModel.getDistanceInfluence: allow null values * remove comment * comment * make CustomWeighting.Parameters constructor public too, graphhopper#2717 * pt: push back mapdb * remove hasDistanceInfluence, graphhopper#2716 * i18n: updated, graphhopper#2721 * cleanup of ElevationProvider, fixes graphhopper#2642 * average_speed path detail: avoid infinity value (or max JS floats) near bridges etc, graphhopper#2636 * fixup: pt: push back mapdb * bug fix for graphhopper#2716 * Add GH version to client-hc * Fix * make custom areas available in custom models (graphhopper#2725) * make it possible to use all custom areas in all custom models * minor comment regarding ID * set feature.id from feature.properties.id for our countries.geojson, related to graphhopper#2725 * getResourceAsStream implicitly creates a FileSystem required when the file is in the jar, https://stackoverflow.com/q/25032716/194609 * specially handle "NO_EDGE"; fixes graphhopper#2723 * Extract method * avoid rounding down small speed (graphhopper#2726) * Maxspeed fix (graphhopper#2727) * clean up speed and maxspeed handling * bug fix * for bike we have to pick maximum max_speed * fix yet another bug * Do not check if edge flags are empty before handling node tags * Return void from TagParser#handleWayTags * Remove some explicit edge flag usage * EncodingManager: initializer config is only needed when building * Fix * Use EV factory * Test access and speed instead of just flags.isEmpty() for foot and bike parsers (graphhopper#2728) * i18n: adding no_NO and updating pl_PL while at it (graphhopper#2730) * areas: don't enforce empty properties, use separate check for areaID, related to graphhopper#2725 * increase point_hint limit from 100 to 170, fixes graphhopper#2732 * Moving Bokmål locale to the correct nb_NO (graphhopper#2731) * i18n: moving Bokmål locale to the correct nb_NO * i18n: fix a missing placeholder in translation and add nb_NO to translationmap * i18n: lexicographical order, graphhopper#2731 * support for foot:backward and forward, fixes graphhopper#2565 * minor docs update * move area/EV name check into relevant classes and fix graphhopper#2735 * fix annoying error message if path details are missing * remove comment (hmm-lib is no longer used) see graphhopper@789580c * Split vehicle tag parsers into access, speed and priority (graphhopper#2738) * Use multi-threading for subnetwork preparation (graphhopper#2737) * bike: always use 4km/h for railway=platform, discussion in graphhopper#2728 * use custom_models.directory consistent naming regarding custom_areas.directory * Report rank (by distance to network) of chosen snaps * Graph speed measurement * Remove block area (graphhopper#2741) * Make sure waypoints are not removed during path simplification (graphhopper#2742) * use FeatureCollection instead of String,Feature Map (graphhopper#2734) * use FeatureCollection instead of String,Feature Map for CustomModel.areas * issue * remove comment * adapt json and docs * fix doc * cleanup * use GH maps from custom_area_feature_collection branch * fix tests * fix graphhopper#2339 (graphhopper#2736) * removed unused method (Polygon, graphhopper#2741) * more mapmatching statistics * Remove no-longer existing extraProfiles field from maps config * i18n: updated sv_SE (graphhopper#2748) * Revert "removed unused method (Polygon, graphhopper#2741)" This reverts commit 95565fd. * Revert "Remove block area (graphhopper#2741)" This reverts commit aa71dc2 * Allow small tolerance for waypoint consistency check, graphhopper#2742 * apparently some kind of rounding (?) is going on between the path point list and the snapped waypoint coordinates so the values are only equal with a certain accuracy * without this fix the GH server can respond with an internal server error (500) and our tests did not cover this so far * handle 'permit' like 'private', fixes graphhopper#2712 (graphhopper#2749) * client-hc: add hints to response * PathDetailsBuilderFactory: avoid modifying request * client-hc: return hints as text * move to more recent java versions * Add request transformer hook to RouteResource * i18n: updated tr and fix typo in nb_NO (graphhopper#2750) * minor fix * client-hc: use Helper.toObject * readme: use jar from maven central * Improve error message when using another vehicle than the ones used for the import, fix graphhopper#2758 * CustomModelParser: more explicit verification of method calls, fixes graphhopper#2743 * remove StringEncodedValue support from custom model due to insufficient usage/testing * Extend Heading Documentation (graphhopper#2714) * Minor changes for heading docs, graphhopper#2714 * Make sure LM approximation is never worse than beeline (graphhopper#2756) * remove hike vehicle (graphhopper#2759) * hike removal * use roads vehicle to allow roads with hike_rating * ensure that hike can use higher sac_scale values than foot, i.e. roads vehicle must be used * Remove block_area (again) (graphhopper#2755) * Fix snap issue (graphhopper#2752) * PathDetailsBuilderFactory: avoid modifying request * snap on edge is sometimes too close to tower node leading to inconsistencies due to GHPoint.equals check in QueryOverlayBuilder * correction only necessary for Position.EDGE * fix test setup * fix repeated test * Add test and comment * snapping must be consistent to avoid test failure * consistent * fix comment --------- Co-authored-by: easbar <easbar.mail@posteo.net> * Extract createSolver methods * add parsers only once if multiple bike/foot profiles * Extract test method * Allow adding node-based CH for profiles with turn costs * protected * further lower priority for sidewalk=no, especially for highways not in safe and preferred set, graphhopper#847 * fix bug in conditional handling, graphhopper#1326 * fix comment * clean up custom models and documentation, graphhopper#2659 * foot vehicle: important fix regarding barrier nodes, bug introduced in graphhopper#2738 * Revert "Allow adding node-based CH for profiles with turn costs" This reverts commit b2b955d. * fix bike2.json reference in config example * try older maven * improve documentation for windows * make mvn deploy to Github Actions working again with a special option instead of a version downgrade * Use way point indices instead of intervals in ResponsePath, graphhopper#2742 * the intervals are only really needed for the simplification, so create them there * Clarify test: instruction for two-lane (!) u-turn * use custom weighting, since this should be most common now * algorithm should not matter (this is tested elsewhere) * make get+setTags of ReaderElement consistent * fix follow up snap issue (graphhopper#2769) * Update snapped point when changing the closest node * update wayindex and position too * update query distance too --------- Co-authored-by: easbar <easbar.mail@posteo.net> * Allow skipping the creation of ReaderNodes/Ways/Relations for pbfs, skip nodes in first pass of OSM import (graphhopper#2770) * Show u-turn instruction for plain u-turns (graphhopper#2767) * 7.0 release - change readme * use 8.0-SNAPSHOT * Add path details for the time, distance and weight of the single legs (graphhopper#2768) * A*: set weight of visited path explicitly * changelog: set release date * Reuse the polygon BBox (graphhopper#2685) Co-authored-by: Thomas Butz <thomas.butz@optitool.de> * Make use of node tags like 'highway' or 'crossing' (graphhopper#2705) * Pass all node tags to edge handlers * Remove node tag count (was only used for analysis) * crossing EV (graphhopper#2706) * add tests; fix traffic_signalS; remove zebra as same as uncontrolled; include proposed tags https://wiki.openstreetmap.org/wiki/Proposed_features/Highway_crossing_cleanup * fix * fix tests * minor fix * minor typo * node tag whitelist; use get not put in removeTag * include node tags handling for road_access and fords (road_environment) * mark only node tags of barrier edge as 'split_node' * avoid stream and collect * minor further perf tune * make barrier edge explicit via tag * simplify handleNodeTags * Remove artificial gh:split_node tag and use long hash set instead * minor rename * use EdgeKVStorage for node tags storage * rename EdgeKVStorage to KVStorage as also used to temporarily store node tags for import * log more infos about node tags; no compact necessary due to KVStorage; limit pointer of KVStorage to positive numbers for now * rename method to handleBarrierEdge * fix taggednodecount method * node tags for the barrier edge should be identical, i.e. use the ref instead of a copy --------- Co-authored-by: Peter <graphhopper@gmx.de> * changelog * pt: remove hardcoded use of FastestWeighting to avoid problems with foot-priority-0 links * New "backward_xy" variable notation in custom models (graphhopper#2760) * initial backward_car_access impl * test+fix when used in CustomModel * new test regarding the addition order of (bike) VehicleTagParser and selected other TagParser * New signature for encoded values and tag parsers (graphhopper#2740) * Add static creators for enum encoded values * Rename createIntAccess -> createEdgeIntAccess * bike profile should allow reverse oneway roads and set walking speed (graphhopper#2774) * bike: allow reverse oneways; mark them as 'get off bike' and slow down to 4km/h * fix speed issue * fix test * create bike_oneway that is always true, except for the reverse direction of a oneway road * fix tests * minor cleanup * initial backward_car_access impl * test+fix when used in CustomModel * use modified bike profile * include custom_models/bike.json in integration test * for bike test reverse access of oneways; use jackson allow comments feature * revert changes to jackson parsing with comment * include bike_priority in bike.json * make custom models simpler via setting bike speed and priority unconditionally at the beginning * add get_off_bike parser after bike_access * comment * fixed typo * re-add test * imports * minor revert to master for 2 files * Never treat a highway tagged with cycleway:both as oneway for bicycles (graphhopper#2776) * include permit->private handling in road_access, graphhopper#2712, graphhopper#2749 * add 4 more enc values to config description * avoid get_off_bike problem for path, fixes graphhopper#2777 * fix bug in bike custom model * Improve bicycle handling: Now prioritise cycleways with SLIGHT_PREFER (graphhopper#2785) * Improve bicycle handling: Now prioritise cycleways with SLIGHT_PREFER, fixes graphhopper#2784 * Add priority handling support for "cycleway:both" i "lane" * Add priority handling support for "cycleway=opposite_track" as discussed in graphhopper#2786. * custom_model_files arrays (graphhopper#2787) * replace custom_model_file with custom_model_files array * fix changelog * certain conditional tags allow to ignore max_weight (graphhopper#2788) * add max_weight_except encoded value; use conditional tagging to set hgv and max_weight; add maxweightrating:hgv parsing for max_weight * include maxweightrating:hgv for MaxWeightExceptParser too * Apply suggestions from code review Co-authored-by: otbutz <tbutz@optitool.de> * Update MaxWeightExceptParser.java * Update OSMMaxWeightParser.java * fix import --------- Co-authored-by: otbutz <tbutz@optitool.de> * Check cycleway access tags (graphhopper#2786) * Check cycleway access * Test cycleway access * Test with non-preferred highway type * Update contributors * removed parser options block_private and block_fords (graphhopper#2780) * removed block_private and block_fords * fix issue number * exclude private for all models under custom_models * make intendedValues and others read-only * adjust benchmarks for graphhopper#2780 * removed motorcycle vehicle (graphhopper#2781) * removed block_private and block_fords * fix issue number * removed motorcycle vehicle * minor fixes * minor change * move curvature part outside of motorcycle * fix config-example.yml * hike and cargo_bike: exclude private roads * custom models: list priority first for no particular reason * minor tweak * Revert "removed parser options block_private and block_fords (graphhopper#2780)" This reverts commit be7209b. * A*: infinite approximation means the target is unreachable * Make the MiniGraphUI working again. (graphhopper#2793) * Use separate logger for OSM warnings * osm_warning -> osm_warnings * update graphhopper maps to latest * maps: avoid URI too long for bigger custom models * avoid IllegalStateException if duplicate path detail * bike custom model: fix roundabout * Prefer bicycle_road/cycleway (graphhopper#2790) * Prefer bicycle roads and cycle streets * Handle pushing section priority * Add unittests * Simplify check * Move test into dedicated method * Update changelog * i18n: update da_DK (graphhopper#2794) * Improve surface parser (graphhopper#2792) * Move synonyms into Surface * Add support for unhewn_cobblestone * Handle surface subtypes * Treat pebblestone as gravel * Treat grass_paver as grass * Add support for wooden surfaces * Test all synonyms * Check subtypes in dedicated test * Update changelog * avoid config mistakes * Add timeout_ms parameter for routing requests (graphhopper#2795) * Upgrade to dropwizard 2.1.6 (graphhopper#2653) * upgrade to dropwizard 2.1.1 * try 2.1.4 * latest 2.1.6 --------- Co-authored-by: easbar <easbar.mail@posteo.net> * resurrect appveyor * appveyor: revert part of the change * upgrade commons-io + junit-bom + osmosis-osm-binary * bike: differentiate between bad highways (graphhopper#2796) * bike: there is a difference between avoid highways like trunk and secondary * add one more test for secondary * avoid creation of values() * reduce calling PriorityCode.getValue, code review in graphhopper#2796 * minor test rename * Update nl and cz translations (graphhopper#2799) * Update nl and cz translations * Fix nl translation * upgrade janino to latest version 3.1.9; remove our workaround for 3.1.6 and below * cleanup of EnumEncodedValue (graphhopper#2800) * remove custom toString in EnumEncodedValue and avoid calling Enum::values() * API JSON response must not change * custom toString necessary for Country and HazmatTunnel * fix imports * always use unix LF, related graphhopper#2791 * Add root editorconfig (graphhopper#2791) * Update CONTRIBUTING.md * Prefer bigger roads for racebike (graphhopper#2802) * Make the MiniGraphUI working again. * Improve priority handling for race bikes as discussed in graphhopper#2796 * Adopt expected Monaco test result for race bike profile * changelog for graphhopper#2796 and graphhopper#2802 * This is a 400 user error, not an ERROR in the logging sense * Simplify WaySegmentParser.Builder * Release some memory earlier after reading OSM * profile name, not vehicle name * Fix * Add check for pillar node overflow * OSMNodeData.nodeKVStorage: add missing create * ferry: hgv=yes allows car even if car tagging is missing * B-tree: Make space for values configurable (graphhopper#2814) * initial working version * make to and from long more clear * make it working for negative keys and values * nextLong(bounds) only for more recent JDKs * fix javadoc * Use long for pillar node id stack variables * use long where it could be a pillar or tower node * simulate large amount of pillar nodes * remove second check for pillar nodes * use 0 as start again * minor * make empty value configurable * changes for review --------- Co-authored-by: easbar <easbar.mail@posteo.net> * clarify foot access parser (graphhopper#2801) * De-prioritise steps for bicycles (graphhopper#2804) * De-prioritise steps for bicycles, fixes graphhopper#2803 original discovery Also use MIN_SPEED for highway=steps in the mtb profile * Don't explicitly call setHighwaySpeed for Mountainbike for values which are used in BikeCommonAverageSpeedParser * De-prioritise steps for bicycles to BAD * Also initialize the avoidHighwayTags with BAD just in order to avoid confusion. * Remove steps from the avoidHighwayTags map * i18n: updated tr (@ihsanguldur) and added new translation for Kazakh kz * differentiate travel time betas for access vs egress * Make sure directed edge filter receives real edge * public CustomModelParser.createWeightingParameters * hike model: allow private tags that are for motor vehicles only, adds tests of the model, fixes graphhopper#2820 * Update README.md * Set empty custom model in custom profile constructor (graphhopper#2821) * Update deploy.md * number formatting: use ENGLISH instead of FRENCH as it sometimes causes missing symbols in terminals * Try latest JDK 20 (graphhopper#2789) * try jdk 20 * try again * Process callables GHUtility.runConcurrently in batches to save memory (graphhopper#2828) * Process callables in batches to save memory * changelog * Do not re-use Executorservice * Overhauled Moving Average Elevation Smoothing (graphhopper#2772) * first implementation which is validated with unit tests no focus on performance, code beauty or documentation. used for validation and comparison with some real world test areas * overwrite previous averaging algorithm, added unit tests and made window size configurable * explain unit test * incorporate pr feedback from otbutz * use IntDoubleHashMap to avoid object unboxing * extended contributor list * pr feedback: rename config to be more consistent * pr feedback: code formatting * pr feedback: having a dedicated class for each implementation and tests * avoid incorrect urban_density classification for the USA (graphhopper#2832) * initial workaround for graphhopper#2829 * use enum * Fix QueryGraph#getEdges * previously we got an error when we tried to call getEdgeIteratorState with edge IDs running up to QueryGraph#getEdges() * Country subdivision (state) (graphhopper#2830) * initial version to include subdivision in Country enum * minor fix * use correct PR * GHUtility cleanup * allow country comparison via getAlpha3 method * minor fixes * clarify iso codes with the help of wikipedia * use alpha2 for custom model to make it more consistent * minor fixes * try separate state instead of country extension * minor enhancement * use complete use for getStateCode * fix * use getter instead of create * remove TODO * for now this method is used only once, so avoid overhead * shorter * Use ForkJoinPool for concurrent processing (graphhopper#2833) * residential should not be ignored but treated as tertiary, graphhopper#2832 * Remove BitUtilBig (graphhopper#2811) * Fix interchanged parameter names in bit util * removed BitUtilBig * cleanup --------- Co-authored-by: Peter <graphhopper@gmx.de> * update GH maps to latest * Clean up country/state selection logic, graphhopper#2830 * default speed limits (graphhopper#2810) * initial version * less output * maxspeed tag vs max_speed name * less logging * use urbanDensity in replaceFunction and explicitly set maxspeed only if UNSET_SPEED * added tests * replace CountryRule.getMaxSpeed with LegalDefaultSpeeds lib * tricky. thinking... * lane support * surface support * residential roads have the same limits in Germany * do not forget the reverse speed * use same version as osm-legal-default-speeds-jvm * something is strange with dropwizard tests as they require an explicit dependency on kotlin-stdlib (not kotlin-stdlib-jdk8) * force okhttp and osm-legal-default-speeds-jvm to use both more recent but same kotlin 1.8.0 * use more recent okio with recommended kotlin 1.6.20; seems to work for osm-legal-default-speeds-jvm too * make createWeightingParameters public * separate storage and parser for default max speed and only combine this after import with the maxspeed tag from OSM * separate close * default speed library does not support maxspeed:forward and :backward tags * test regarding living_street is now useless at that level * comment * fix problematic forward/backward case * store osm max_speed and temporarily the default urban+rural from library to improve max_speed afterwards * reduce overhead a bit and force type * grow array on get too * storage: we need a tiny offset * okhttp 4.11.0 has cleaner deps * reduce tag count and cache speeds to reduce overhead, see westnordost/osm-legal-default-speeds#7 * configure preliminary GHDirectory * added maxweight * avoid default-maxspeed library while OSM parsing * Revert "avoid default-maxspeed library while OSM parsing" This reverts commit 8b07594. * fix setup to use the same GHDirectory instance to avoid problems with missing directory etc * consider walk as 6kmh * don't interpret country-dependent speeds in stringToKmh * call computeIfAbsent for cache; convert maxspeed into kmh before import; fix test for stringToKmh * fix comment * added description to config * add max_speed_estimated boolean encoded value * use speedlib v1.3 * use statecode to feed speedlib * updated legal_default_speeds.json * residential should not be ignored but treated as tertiary, graphhopper#2832 * comment * Update maps * typo * bike: remove hazmat handling, fixes graphhopper#2840 * bug fix for UK speed limit * use unlimited speed constant for DEU * Fix compilation * Fix matrix test * Fix tests * Fix compilation * Fix matrix profiles and add tests * Fix merge conflict --------- Co-authored-by: Peter <graphhopper@gmx.de> Co-authored-by: ratrun <ratrun@gmx.at> Co-authored-by: easbar <easbar.mail@posteo.net> Co-authored-by: Michael Zilske <michael.zilske@tu-berlin.de> Co-authored-by: OlafFlebbeBosch <123375381+OlafFlebbeBosch@users.noreply.github.com> Co-authored-by: Lukas Weber <32765578+lukasalexanderweber@users.noreply.github.com> Co-authored-by: otbutz <tbutz@optitool.de> Co-authored-by: Thomas Butz <thomas.butz@optitool.de> Co-authored-by: bt90 <btom1990@googlemail.com> Co-authored-by: Robin <boldtrn@users.noreply.github.com> Co-authored-by: Christoph Lingg <christoph@lingg.eu>

karussell added 2 commits January 6, 2023 17:40

distance_influence should only be sent to server when explicitly spec…

2091d33

…ified and the default value of 70 makes this impossible

fix changelog

1a32582

karussell temporarily deployed to benchmarks January 6, 2023 16:48 — with GitHub Actions Inactive

changelog

3a443b5

karussell temporarily deployed to benchmarks January 6, 2023 17:03 — with GitHub Actions Inactive

karussell added the refactoring label Jan 6, 2023

karussell added this to the 7.0 milestone Jan 6, 2023

karussell changed the title ~~distance_influence should only be sent to server when explicitly specified~~ client-hc: distance_influence should only be sent to server when explicitly specified Jan 6, 2023

easbar reviewed Jan 6, 2023

View reviewed changes

karussell changed the title ~~client-hc: distance_influence should only be sent to server when explicitly specified~~ distance_influence should not get a default value if no value is specified Jan 6, 2023

easbar reviewed Jan 9, 2023

View reviewed changes

CustomModel.getDistanceInfluence: allow null values

8ae8077

karussell temporarily deployed to benchmarks January 9, 2023 19:54 — with GitHub Actions Inactive

easbar reviewed Jan 9, 2023

View reviewed changes

core/src/test/java/com/graphhopper/routing/weighting/custom/CustomWeightingTest.java Show resolved Hide resolved

easbar reviewed Jan 9, 2023

View reviewed changes

web-api/src/main/java/com/graphhopper/util/CustomModel.java Outdated Show resolved Hide resolved

easbar reviewed Jan 9, 2023

View reviewed changes

remove comment

945f484

karussell temporarily deployed to benchmarks January 10, 2023 08:08 — with GitHub Actions Inactive

karussell merged commit 1ea0a66 into master Jan 10, 2023

karussell deleted the remove_default_distance_influence branch January 10, 2023 08:23

easbar reviewed Jan 10, 2023

View reviewed changes

karussell added a commit that referenced this pull request Jan 12, 2023

remove hasDistanceInfluence, #2716

d3255a6

karussell added a commit that referenced this pull request Jan 14, 2023

bug fix for #2716

e1e34c2

akuracy pushed a commit to s3pweb/graphhopper that referenced this pull request Mar 21, 2023

remove hasDistanceInfluence, graphhopper#2716

954eb74

akuracy pushed a commit to s3pweb/graphhopper that referenced this pull request Mar 21, 2023

bug fix for graphhopper#2716

ddffa62

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

distance_influence should not get a default value if no value is specified #2716

distance_influence should not get a default value if no value is specified #2716

karussell commented Jan 6, 2023 •

edited

easbar Jan 6, 2023

karussell Jan 6, 2023

easbar Jan 9, 2023

karussell Jan 9, 2023

easbar commented Jan 6, 2023

karussell commented Jan 6, 2023

easbar Jan 9, 2023

karussell Jan 9, 2023

easbar Jan 9, 2023

easbar Jan 9, 2023

karussell Jan 10, 2023 •

edited

otbutz Jan 10, 2023

easbar Jan 10, 2023

easbar Jan 10, 2023

otbutz Jan 10, 2023

easbar Jan 10, 2023

easbar Jan 10, 2023

easbar Jan 10, 2023

karussell Jan 10, 2023 •

edited

easbar Jan 10, 2023

easbar Jan 10, 2023

otbutz commented Jan 10, 2023

karussell commented Jan 10, 2023

easbar commented Jan 10, 2023

karussell commented Jan 10, 2023

		- users of client-hc now have to set the distanceInfluence to 70 if a lower value is defined on the server-side and can remove it if they want to match the server-side value which is highly recommended for performance reasons, see #2716
		- there is a new, required 'import.osm.ignored_highways' configuration option that must be used to not increase the graph size and decrease performance for motorized-only routing compared to previous versions, #2702

		double qmDI = queryModel.hasDistanceInfluence() ? queryModel.getDistanceInfluence() : 0;
		double bmDI = baseModel.hasDistanceInfluence() ? baseModel.getDistanceInfluence() : 0;

distance_influence should not get a default value if no value is specified #2716

distance_influence should not get a default value if no value is specified #2716

Conversation

karussell commented Jan 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easbar commented Jan 6, 2023

karussell commented Jan 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karussell Jan 10, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karussell Jan 10, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

otbutz commented Jan 10, 2023

karussell commented Jan 10, 2023

easbar commented Jan 10, 2023

karussell commented Jan 10, 2023

karussell commented Jan 6, 2023 •

edited

karussell Jan 10, 2023 •

edited

karussell Jan 10, 2023 •

edited