From a9b1bd88d3073d2a23503fe4f43f615f870555e7 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Thu, 30 May 2024 17:13:44 +0200 Subject: [PATCH] Remove all core-CH left-overs (#6920) * Remove all core-CH left-overs * Fix formatting * Update CHANGELOG.md --- CHANGELOG.md | 1 + docs/nodejs/api.md | 2 +- include/engine/engine_config.hpp | 7 +----- include/engine/search_engine_data.hpp | 1 - include/nodejs/node_osrm_support.hpp | 9 ++----- scripts/ci/windows-build.bat | 6 ----- src/nodejs/node_osrm.cpp | 2 +- src/osrm/osrm.cpp | 7 ------ src/tools/routed.cpp | 4 +-- test/data/Makefile | 17 ++----------- test/nodejs/constants.js | 2 -- test/nodejs/index.js | 21 +++------------- test/nodejs/route.js | 27 -------------------- unit_tests/library/algorithm.cpp | 8 ------ unit_tests/library/options.cpp | 10 -------- unit_tests/library/tile.cpp | 36 --------------------------- 16 files changed, 13 insertions(+), 147 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbeaf91df..965c2a3d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - Changes from 5.27.1 - Features + - REMOVED: Remove all core-CH left-overs [#6920](https://github.com/Project-OSRM/osrm-backend/pull/6920) - ADDED: Add support for a keepalive_timeout flag. [#6674](https://github.com/Project-OSRM/osrm-backend/pull/6674) - ADDED: Add support for a default_radius flag. [#6575](https://github.com/Project-OSRM/osrm-backend/pull/6575) - ADDED: Add support for disabling feature datasets. [#6666](https://github.com/Project-OSRM/osrm-backend/pull/6666) diff --git a/docs/nodejs/api.md b/docs/nodejs/api.md index 9dbfe912c..a87452cbc 100644 --- a/docs/nodejs/api.md +++ b/docs/nodejs/api.md @@ -21,7 +21,7 @@ var osrm = new OSRM('network.osrm'); **Parameters** - `options` **([Object](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object) \| [String](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String))** Options for creating an OSRM object or string to the `.osrm` file. (optional, default `{shared_memory:true}`) - - `options.algorithm` **[String](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String)?** The algorithm to use for routing. Can be 'CH', 'CoreCH' or 'MLD'. Default is 'CH'. + - `options.algorithm` **[String](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String)?** The algorithm to use for routing. Can be 'CH', or 'MLD'. Default is 'CH'. Make sure you prepared the dataset with the correct toolchain. - `options.shared_memory` **[Boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** Connects to the persistent shared memory datastore. This requires you to run `osrm-datastore` prior to creating an `OSRM` object. diff --git a/include/engine/engine_config.hpp b/include/engine/engine_config.hpp index c7c7eb06f..cb986be9e 100644 --- a/include/engine/engine_config.hpp +++ b/include/engine/engine_config.hpp @@ -54,14 +54,10 @@ namespace osrm::engine * * In addition, shared memory can be used for datasets loaded with osrm-datastore. * - * You can chose between three algorithms: + * You can chose between two algorithms: * - Algorithm::CH * Contraction Hierarchies, extremely fast queries but slow pre-processing. The default right * now. - * - Algorithm::CoreCH - * Deprecated, to be removed in v6.0 - * Contraction Hierachies with partial contraction for faster pre-processing but slower - * queries. * - Algorithm::MLD * Multi Level Dijkstra, moderately fast in both pre-processing and query. * @@ -74,7 +70,6 @@ struct EngineConfig final enum class Algorithm { CH, - CoreCH, // Deprecated, will be removed in v6.0 MLD }; diff --git a/include/engine/search_engine_data.hpp b/include/engine/search_engine_data.hpp index 4060ab6b2..16ffa25b1 100644 --- a/include/engine/search_engine_data.hpp +++ b/include/engine/search_engine_data.hpp @@ -12,7 +12,6 @@ namespace osrm::engine // Algorithm-dependent heaps // - CH algorithms use CH heaps -// - CoreCH algorithms use CH // - MLD algorithms use MLD heaps template struct SearchEngineData diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index 18fb20bac..cd0043bb4 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -296,24 +296,19 @@ inline engine_config_ptr argumentsToEngineConfig(const Napi::CallbackInfo &args) { engine_config->algorithm = osrm::EngineConfig::Algorithm::CH; } - else if (algorithm_str == "CoreCH") - { - engine_config->algorithm = osrm::EngineConfig::Algorithm::CH; - } else if (algorithm_str == "MLD") { engine_config->algorithm = osrm::EngineConfig::Algorithm::MLD; } else { - ThrowError(args.Env(), "algorithm option must be one of 'CH', 'CoreCH', or 'MLD'."); + ThrowError(args.Env(), "algorithm option must be one of 'CH', or 'MLD'."); return engine_config_ptr(); } } else if (!algorithm.IsUndefined()) { - ThrowError(args.Env(), - "algorithm option must be a string and one of 'CH', 'CoreCH', or 'MLD'."); + ThrowError(args.Env(), "algorithm option must be a string and one of 'CH', or 'MLD'."); return engine_config_ptr(); } diff --git a/scripts/ci/windows-build.bat b/scripts/ci/windows-build.bat index 71c9bfc71..bc1fc0a8f 100644 --- a/scripts/ci/windows-build.bat +++ b/scripts/ci/windows-build.bat @@ -58,7 +58,6 @@ IF %ERRORLEVEL% NEQ 0 GOTO ERROR SET test_region=monaco SET test_region_ch=ch\monaco -SET test_region_corech=corech\monaco SET test_region_mld=mld\monaco SET test_osm=%test_region%.osm.pbf COPY %PROJECT_DIR%\test\data\%test_region%.osm.pbf %test_osm% @@ -68,18 +67,13 @@ IF %ERRORLEVEL% NEQ 0 GOTO ERROR MKDIR ch XCOPY %test_region%.osrm.* ch\ XCOPY %test_region%.osrm ch\ -MKDIR corech -XCOPY %test_region%.osrm.* corech\ -XCOPY %test_region%.osrm corech\ MKDIR mld XCOPY %test_region%.osrm.* mld\ XCOPY %test_region%.osrm mld\ %CONFIGURATION%\osrm-contract.exe %test_region_ch%.osrm -%CONFIGURATION%\osrm-contract.exe --core 0.8 %test_region_corech%.osrm %CONFIGURATION%\osrm-partition.exe %test_region_mld%.osrm %CONFIGURATION%\osrm-customize.exe %test_region_mld%.osrm XCOPY /Y ch\*.* ..\test\data\ch\ -XCOPY /Y corech\*.* ..\test\data\corech\ XCOPY /Y mld\*.* ..\test\data\mld\ unit_tests\%CONFIGURATION%\library-tests.exe IF %ERRORLEVEL% NEQ 0 GOTO ERROR diff --git a/src/nodejs/node_osrm.cpp b/src/nodejs/node_osrm.cpp index 65585abac..5971fec1c 100644 --- a/src/nodejs/node_osrm.cpp +++ b/src/nodejs/node_osrm.cpp @@ -64,7 +64,7 @@ Napi::Object Engine::Init(Napi::Env env, Napi::Object exports) * ``` * * @param {Object|String} [options={shared_memory: true}] Options for creating an OSRM object or string to the `.osrm` file. - * @param {String} [options.algorithm] The algorithm to use for routing. Can be 'CH', 'CoreCH' or 'MLD'. Default is 'CH'. + * @param {String} [options.algorithm] The algorithm to use for routing. Can be 'CH', or 'MLD'. Default is 'CH'. * Make sure you prepared the dataset with the correct toolchain. * @param {Boolean} [options.shared_memory] Connects to the persistent shared memory datastore. * This requires you to run `osrm-datastore` prior to creating an `OSRM` object. diff --git a/src/osrm/osrm.cpp b/src/osrm/osrm.cpp index cf961d5fb..7f0ab1034 100644 --- a/src/osrm/osrm.cpp +++ b/src/osrm/osrm.cpp @@ -36,13 +36,6 @@ OSRM::OSRM(engine::EngineConfig &config) // Now, check that the algorithm requested can be used with the data // that's available. - - if (config.algorithm == EngineConfig::Algorithm::CoreCH) - { - util::Log(logWARNING) << "Using CoreCH is deprecated. Falling back to CH"; - config.algorithm = EngineConfig::Algorithm::CH; - } - switch (config.algorithm) { case EngineConfig::Algorithm::CH: diff --git a/src/tools/routed.cpp b/src/tools/routed.cpp index 4f2fae5ef..f4c229cba 100644 --- a/src/tools/routed.cpp +++ b/src/tools/routed.cpp @@ -61,7 +61,7 @@ std::istream &operator>>(std::istream &in, EngineConfig::Algorithm &algorithm) in >> token; boost::to_lower(token); - if (token == "ch" || token == "corech") + if (token == "ch") algorithm = EngineConfig::Algorithm::CH; else if (token == "mld") algorithm = EngineConfig::Algorithm::MLD; @@ -159,7 +159,7 @@ inline unsigned generateServerProgramOptions(const int argc, ("algorithm,a", value(&config.algorithm) ->default_value(EngineConfig::Algorithm::CH, "CH"), - "Algorithm to use for the data. Can be CH, CoreCH, MLD.") // + "Algorithm to use for the data. Can be CH, MLD.") // ("disable-feature-dataset", value>(&config.disable_feature_dataset)->multitoken(), "Disables a feature dataset from being loaded into memory if not needed. Options: " diff --git a/test/data/Makefile b/test/data/Makefile index 14c586e93..2c617405b 100755 --- a/test/data/Makefile +++ b/test/data/Makefile @@ -14,20 +14,16 @@ PROFILE:=$(PROFILE_ROOT)/car.lua all: data -data: ch/$(DATA_NAME).osrm.hsgr corech/$(DATA_NAME).osrm.hsgr mld/$(DATA_NAME).osrm.partition +data: ch/$(DATA_NAME).osrm.hsgr mld/$(DATA_NAME).osrm.partition clean: -rm -r $(DATA_NAME).* - -rm -r ch corech mld + -rm -r ch mld ch/$(DATA_NAME).osrm: $(DATA_NAME).osrm mkdir -p ch cp $(DATA_NAME).osrm.* ch/ -corech/$(DATA_NAME).osrm: $(DATA_NAME).osrm - mkdir -p corech - cp $(DATA_NAME).osrm.* corech/ - mld/$(DATA_NAME).osrm: $(DATA_NAME).osrm mkdir -p mld cp $(DATA_NAME).osrm.* mld/ @@ -42,10 +38,6 @@ ch/$(DATA_NAME).osrm.hsgr: ch/$(DATA_NAME).osrm $(PROFILE) $(OSRM_CONTRACT) @echo "Running osrm-contract..." $(TIMER) "osrm-contract\t$@" $(OSRM_CONTRACT) $< -corech/$(DATA_NAME).osrm.hsgr: corech/$(DATA_NAME).osrm $(PROFILE) $(OSRM_CONTRACT) - @echo "Running osrm-contract..." - $(TIMER) "osrm-contract\t$@" $(OSRM_CONTRACT) --core=0.5 $< - mld/$(DATA_NAME).osrm.partition: mld/$(DATA_NAME).osrm $(PROFILE) $(OSRM_PARTITION) @echo "Running osrm-partition..." $(TIMER) "osrm-partition\t$@" $(OSRM_PARTITION) $< @@ -61,11 +53,6 @@ benchmark: data $(DATA_NAME).requests $(TIMER) "queries\tCH" "cat $(DATA_NAME).requests | xargs curl &> /dev/null" @cat osrm-routed.pid | xargs kill @rm osrm-routed.pid - @/bin/sh -c '$(OSRM_ROUTED) --algorithm=CoreCH corech/$(DATA_NAME).osrm > /dev/null & echo "$$!" > osrm-routed.pid' - @sleep 1 - $(TIMER) "queries\tCoreCH" "cat $(DATA_NAME).requests | xargs curl &> /dev/null" - @cat osrm-routed.pid | xargs kill - @rm osrm-routed.pid @/bin/sh -c '$(OSRM_ROUTED) --algorithm=MLD mld/$(DATA_NAME).osrm > /dev/null & echo "$$!" > osrm-routed.pid' @sleep 1 $(TIMER) "queries\tMLD" "cat $(DATA_NAME).requests | xargs curl &> /dev/null" diff --git a/test/nodejs/constants.js b/test/nodejs/constants.js index e56742e0c..e62b46254 100644 --- a/test/nodejs/constants.js +++ b/test/nodejs/constants.js @@ -16,12 +16,10 @@ exports.test_tile = {'at': [17059, 11948, 15], 'size': 159125}; if (process.env.OSRM_DATA_PATH !== undefined) { exports.data_path = path.join(path.resolve(process.env.OSRM_DATA_PATH), "ch/monaco.osrm"); exports.mld_data_path = path.join(path.resolve(process.env.OSRM_DATA_PATH), "mld/monaco.osrm"); - exports.corech_data_path = path.join(path.resolve(process.env.OSRM_DATA_PATH), "corech/monaco.osrm"); exports.test_memory_path = path.join(path.resolve(process.env.OSRM_DATA_PATH), "test_memory"); console.log('Setting custom data path to ' + exports.data_path); } else { exports.data_path = path.resolve(path.join(__dirname, "../data/ch/monaco.osrm")); exports.mld_data_path = path.resolve(path.join(__dirname, "../data/mld/monaco.osrm")); - exports.corech_data_path = path.resolve(path.join(__dirname, "../data/corech/monaco.osrm")); exports.test_memory_path = path.resolve(path.join(__dirname, "../data/test_memory")); } diff --git a/test/nodejs/index.js b/test/nodejs/index.js index 73667cb8c..88a516384 100644 --- a/test/nodejs/index.js +++ b/test/nodejs/index.js @@ -3,7 +3,6 @@ var test = require('tape'); var monaco_path = require('./constants').data_path; var test_memory_file = require('./constants').test_memory_file; var monaco_mld_path = require('./constants').mld_data_path; -var monaco_corech_path = require('./constants').corech_data_path; test('constructor: throws if new keyword is not used', function(assert) { assert.plan(1); @@ -65,13 +64,13 @@ test('constructor: throws if given a non-string/obj argument', function(assert) test('constructor: throws if given an unkown algorithm', function(assert) { assert.plan(1); assert.throws(function() { new OSRM({algorithm: 'Foo', shared_memory: true}); }, - /algorithm option must be one of 'CH', 'CoreCH', or 'MLD'/); + /algorithm option must be one of 'CH', or 'MLD'/); }); test('constructor: throws if given an invalid algorithm', function(assert) { assert.plan(1); assert.throws(function() { new OSRM({algorithm: 3, shared_memory: true}); }, - /algorithm option must be a string and one of 'CH', 'CoreCH', or 'MLD'/); + /algorithm option must be a string and one of 'CH', or 'MLD'/); }); test('constructor: loads MLD if given as algorithm', function(assert) { @@ -86,22 +85,8 @@ test('constructor: loads CH if given as algorithm', function(assert) { assert.ok(osrm); }); -test('constructor: loads CoreCH if given as algorithm', function(assert) { - assert.plan(1); - var osrm = new OSRM({algorithm: 'CoreCH', path: monaco_corech_path}); - assert.ok(osrm); -}); - -test('constructor: autoswitches to CoreCH for a CH dataset if capable', function(assert) { - assert.plan(1); - var osrm = new OSRM({algorithm: 'CH', path: monaco_corech_path}); - assert.ok(osrm); -}); - test('constructor: throws if data doesn\'t match algorithm', function(assert) { - assert.plan(3); - assert.throws(function() { new OSRM({algorithm: 'CoreCH', path: monaco_mld_path}); }, /Could not find any metrics for CH/, 'CoreCH with MLD data'); - assert.ok(new OSRM({algorithm: 'CoreCH', path: monaco_path}), 'CoreCH with CH data'); + assert.plan(1); assert.throws(function() { new OSRM({algorithm: 'MLD', path: monaco_path}); }, /Could not find any metrics for MLD/, 'MLD with CH data'); }); diff --git a/test/nodejs/route.js b/test/nodejs/route.js index f84ced4e0..8678cb12a 100644 --- a/test/nodejs/route.js +++ b/test/nodejs/route.js @@ -2,7 +2,6 @@ var OSRM = require('../../'); var test = require('tape'); var monaco_path = require('./constants').data_path; var monaco_mld_path = require('./constants').mld_data_path; -var monaco_corech_path = require('./constants').corech_data_path; var three_test_coordinates = require('./constants').three_test_coordinates; var two_test_coordinates = require('./constants').two_test_coordinates; const flatbuffers = require('../../features/support/flatbuffers').flatbuffers; @@ -76,32 +75,6 @@ test('route: routes Monaco on MLD', function(assert) { }); }); -test('route: routes Monaco on CoreCH', function(assert) { - assert.plan(5); - var osrm = new OSRM({path: monaco_corech_path, algorithm: 'CoreCH'}); - osrm.route({coordinates: [[13.43864,52.51993],[13.415852,52.513191]]}, function(err, route) { - assert.ifError(err); - assert.ok(route.waypoints); - assert.ok(route.routes); - assert.ok(route.routes.length); - assert.ok(route.routes[0].geometry); - }); -}); - -test('route: routes Monaco and returns a JSON buffer', function(assert) { - assert.plan(6); - var osrm = new OSRM({path: monaco_corech_path, algorithm: 'CoreCH'}); - osrm.route({coordinates: [[13.43864,52.51993],[13.415852,52.513191]]}, { format: 'json_buffer'}, function(err, result) { - assert.ifError(err); - assert.ok(result instanceof Buffer); - const route = JSON.parse(result); - assert.ok(route.waypoints); - assert.ok(route.routes); - assert.ok(route.routes.length); - assert.ok(route.routes[0].geometry); - }); -}); - test('route: throws with too few or invalid args', function(assert) { assert.plan(4); var osrm = new OSRM(monaco_path); diff --git a/unit_tests/library/algorithm.cpp b/unit_tests/library/algorithm.cpp index b22cbfba6..31798ffb3 100644 --- a/unit_tests/library/algorithm.cpp +++ b/unit_tests/library/algorithm.cpp @@ -14,14 +14,6 @@ BOOST_AUTO_TEST_CASE(test_incompatible_with_mld) osrm::exception); } -BOOST_AUTO_TEST_CASE(test_compatible_with_corech_fallback) -{ - // Note - this tests that given the CoreCH algorithm config option, configuration falls back to - // CH and is compatible with CH data - BOOST_CHECK_NO_THROW( - getOSRM(OSRM_TEST_DATA_DIR "/ch/monaco.osrm", osrm::EngineConfig::Algorithm::CoreCH)); -} - BOOST_AUTO_TEST_CASE(test_incompatible_with_ch) { // Can't use the CH algorithm with MLD data diff --git a/unit_tests/library/options.cpp b/unit_tests/library/options.cpp index 944a62fc5..2d28e474a 100644 --- a/unit_tests/library/options.cpp +++ b/unit_tests/library/options.cpp @@ -18,16 +18,6 @@ BOOST_AUTO_TEST_CASE(test_ch) OSRM osrm{config}; } -BOOST_AUTO_TEST_CASE(test_corech) -{ - using namespace osrm; - EngineConfig config; - config.use_shared_memory = false; - config.storage_config = storage::StorageConfig(OSRM_TEST_DATA_DIR "/corech/monaco.osrm"); - config.algorithm = EngineConfig::Algorithm::CoreCH; - OSRM osrm{config}; -} - BOOST_AUTO_TEST_CASE(test_mld) { using namespace osrm; diff --git a/unit_tests/library/tile.cpp b/unit_tests/library/tile.cpp index 703e7755b..89d7f1680 100644 --- a/unit_tests/library/tile.cpp +++ b/unit_tests/library/tile.cpp @@ -198,18 +198,6 @@ void test_tile_ch(bool use_string_only_api) BOOST_AUTO_TEST_CASE(test_tile_ch_old_api) { test_tile_ch(true); } BOOST_AUTO_TEST_CASE(test_tile_ch_new_api) { test_tile_ch(false); } -void test_tile_corech(bool use_string_only_api) -{ - // Note: this tests that given the CoreCH algorithm config option, configuration falls back to - // CH and is compatible with CH data - using namespace osrm; - auto osrm = - getOSRM(OSRM_TEST_DATA_DIR "/ch/monaco.osrm", osrm::EngineConfig::Algorithm::CoreCH); - validate_tile(osrm, use_string_only_api); -} -BOOST_AUTO_TEST_CASE(test_tile_corech_old_api) { test_tile_corech(true); } -BOOST_AUTO_TEST_CASE(test_tile_corech_new_api) { test_tile_corech(false); } - void test_tile_mld(bool use_string_only_api) { using namespace osrm; @@ -347,14 +335,6 @@ BOOST_AUTO_TEST_CASE(test_tile_turns_ch_new_api) { test_tile_turns_ch(osrm::EngineConfig::Algorithm::CH, false); } -BOOST_AUTO_TEST_CASE(test_tile_turns_corech_old_api) -{ - test_tile_turns_ch(osrm::EngineConfig::Algorithm::CoreCH, true); -} -BOOST_AUTO_TEST_CASE(test_tile_turns_corech_new_api) -{ - test_tile_turns_ch(osrm::EngineConfig::Algorithm::CoreCH, false); -} void test_tile_turns_mld(bool use_string_only_api) { @@ -432,14 +412,6 @@ BOOST_AUTO_TEST_CASE(test_tile_speeds_ch_new_api) { test_tile_speeds_ch(osrm::EngineConfig::Algorithm::CH, false); } -BOOST_AUTO_TEST_CASE(test_tile_speeds_corech_old_api) -{ - test_tile_speeds_ch(osrm::EngineConfig::Algorithm::CoreCH, true); -} -BOOST_AUTO_TEST_CASE(test_tile_speeds_corech_new_api) -{ - test_tile_speeds_ch(osrm::EngineConfig::Algorithm::CoreCH, false); -} void test_tile_speeds_mld(bool use_string_only_api) { @@ -501,14 +473,6 @@ BOOST_AUTO_TEST_CASE(test_tile_node_ch_new_api) { test_tile_nodes_ch(osrm::EngineConfig::Algorithm::CH, false); } -BOOST_AUTO_TEST_CASE(test_tile_node_corech_old_api) -{ - test_tile_nodes_ch(osrm::EngineConfig::Algorithm::CoreCH, true); -} -BOOST_AUTO_TEST_CASE(test_tile_node_corech_new_api) -{ - test_tile_nodes_ch(osrm::EngineConfig::Algorithm::CoreCH, false); -} void test_tile_nodes_mld(bool use_string_only_api) {