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

Allow -1.0 as unlimited for default_radius value #6599

Merged
merged 5 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
- NodeJS:
- CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
- Misc:
- ADDED: Add support for "unlimited" to be passed as a value for the default-radius and max-matching-radius flags. [#6599](https://github.com/Project-OSRM/osrm-backend/pull/6599)
- CHANGED: Allow -1.0 as unlimited for default_radius value. [#6599](https://github.com/Project-OSRM/osrm-backend/pull/6599)
- CHANGED: keep libosrm* in the docker image for downstream linking [#6602](https://github.com/Project-OSRM/osrm-backend/pull/6602)
- CHANGED: Move vector in CSVFilesParser instead copying it. [#6470](https://github.com/Project-OSRM/osrm-backend/pull/6470)
- REMOVED: Get rid of unused functions in util/json_util.hpp. [#6446](https://github.com/Project-OSRM/osrm-backend/pull/6446)
Expand Down
2 changes: 1 addition & 1 deletion include/engine/engine_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ struct EngineConfig final
int max_locations_map_matching = -1;
double max_radius_map_matching = -1.0;
int max_results_nearest = -1;
boost::optional<double> default_radius;
boost::optional<double> default_radius = -1.0;
int max_alternatives = 3; // set an arbitrary upper bound; can be adjusted by user
bool use_shared_memory = true;
boost::filesystem::path memory_file;
Expand Down
5 changes: 3 additions & 2 deletions include/engine/geospatial_query.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
[this, &max_distance, &max_results, input_coordinate](const std::size_t num_results,
const CandidateSegment &segment) {
return (max_results && num_results >= *max_results) ||
(max_distance &&
(max_distance && max_distance != -1.0 &&
CheckSegmentDistance(input_coordinate, segment, *max_distance));
});

Expand Down Expand Up @@ -163,7 +163,8 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
auto distance = GetSegmentDistance(input_coordinate, segment);
auto further_than_big_component = distance > big_component_distance;
auto no_more_candidates = has_big_component && further_than_big_component;
auto too_far_away = max_distance && distance > *max_distance;
auto too_far_away =
max_distance && max_distance != -1.0 && distance > *max_distance;

// Time to terminate the search when:
// 1. We've found a node from a big component and the next candidate is further away
Expand Down
18 changes: 16 additions & 2 deletions include/nodejs/node_osrm_support.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,16 @@ inline engine_config_ptr argumentsToEngineConfig(const Napi::CallbackInfo &args)
ThrowError(args.Env(), "max_alternatives must be an integral number");
return engine_config_ptr();
}
if (!default_radius.IsUndefined() && !default_radius.IsNumber())
if (!max_radius_map_matching.IsUndefined() && max_radius_map_matching.IsString() &&
max_radius_map_matching.ToString().Utf8Value() != "unlimited")
{
ThrowError(args.Env(), "default_radius must be an integral number");
ThrowError(args.Env(), "max_radius_map_matching must be unlimited or an integral number");
return engine_config_ptr();
}
if (!default_radius.IsUndefined() && default_radius.IsString() &&
default_radius.ToString().Utf8Value() != "unlimited")
{
ThrowError(args.Env(), "default_radius must be unlimited or an integral number");
return engine_config_ptr();
}

Expand All @@ -337,10 +344,17 @@ inline engine_config_ptr argumentsToEngineConfig(const Napi::CallbackInfo &args)
engine_config->max_results_nearest = max_results_nearest.ToNumber().Int32Value();
if (max_alternatives.IsNumber())
engine_config->max_alternatives = max_alternatives.ToNumber().Int32Value();

if (max_radius_map_matching.IsNumber())
engine_config->max_radius_map_matching = max_radius_map_matching.ToNumber().DoubleValue();
else if (max_radius_map_matching.IsString() &&
max_radius_map_matching.ToString().Utf8Value() == "unlimited")
engine_config->max_radius_map_matching = -1.0;

if (default_radius.IsNumber())
engine_config->default_radius = default_radius.ToNumber().DoubleValue();
else if (default_radius.IsString() && default_radius.ToString().Utf8Value() == "unlimited")
engine_config->default_radius = -1.0;

return engine_config;
}
Expand Down
16 changes: 9 additions & 7 deletions src/engine/engine_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ bool EngineConfig::IsValid() const
return v == -1 || v > limit;
};

const bool limits_valid = unlimited_or_more_than(max_locations_distance_table, 2) &&
unlimited_or_more_than(max_locations_map_matching, 2) &&
unlimited_or_more_than(max_radius_map_matching, 0) &&
unlimited_or_more_than(max_locations_trip, 2) &&
unlimited_or_more_than(max_locations_viaroute, 2) &&
unlimited_or_more_than(max_results_nearest, 0) &&
max_alternatives >= 0;
const bool limits_valid =
unlimited_or_more_than(max_locations_distance_table, 2) &&
unlimited_or_more_than(max_locations_map_matching, 2) &&
unlimited_or_more_than(max_radius_map_matching, 0) &&
unlimited_or_more_than(max_locations_trip, 2) &&
unlimited_or_more_than(max_locations_viaroute, 2) &&
unlimited_or_more_than(max_results_nearest, 0) &&
(!default_radius.has_value() || unlimited_or_more_than(*default_radius, 0)) &&
max_alternatives >= 0;

return ((use_shared_memory && all_path_are_empty) || (use_mmap && storage_config.IsValid()) ||
storage_config.IsValid()) &&
Expand Down
4 changes: 2 additions & 2 deletions src/engine/plugins/match.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ Status MatchPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
if (tidied.parameters.radiuses.empty())
{
search_radiuses.resize(tidied.parameters.coordinates.size(),
default_radius.has_value()
default_radius.has_value() && *default_radius != -1.0
? *default_radius
: routing_algorithms::DEFAULT_GPS_PRECISION * RADIUS_MULTIPLIER);
}
Expand All @@ -199,7 +199,7 @@ Status MatchPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
}
else
{
return default_radius.has_value()
return default_radius.has_value() && *default_radius != -1.0
? *default_radius
: routing_algorithms::DEFAULT_GPS_PRECISION * RADIUS_MULTIPLIER;
}
Expand Down
30 changes: 29 additions & 1 deletion src/tools/routed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <boost/algorithm/string/case_conv.hpp>
#include <boost/any.hpp>
#include <boost/filesystem.hpp>
#include <boost/optional/optional_io.hpp>
#include <boost/program_options.hpp>

#include <cstdlib>
Expand Down Expand Up @@ -70,6 +71,33 @@ std::istream &operator>>(std::istream &in, EngineConfig::Algorithm &algorithm)
}
} // namespace osrm::engine

// overload validate for the double type to allow "unlimited" as an input
namespace boost
{
void validate(boost::any &v, const std::vector<std::string> &values, double *, double)
{
boost::program_options::validators::check_first_occurrence(v);
const std::string &s = boost::program_options::validators::get_single_string(values);

if (s == "unlimited")
{
v = -1.0;
}
else
{
try
{
v = std::stod(s);
}
catch (const std::invalid_argument &)
{
throw boost::program_options::validation_error(
boost::program_options::validation_error::invalid_option_value);
}
}
}
} // namespace boost

// generate boost::program_options object for the routing part
inline unsigned generateServerProgramOptions(const int argc,
const char *argv[],
Expand Down Expand Up @@ -149,7 +177,7 @@ inline unsigned generateServerProgramOptions(const int argc,
value<double>(&config.max_radius_map_matching)->default_value(-1.0),
"Max. radius size supported in map matching query. Default: unlimited.") //
("default-radius",
value<boost::optional<double>>(&config.default_radius),
value<boost::optional<double>>(&config.default_radius)->default_value(-1.0),
"Default radius size for queries. Default: unlimited.");

// hidden options, will be allowed on command line, but will not be shown to the user
Expand Down
15 changes: 12 additions & 3 deletions test/nodejs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,17 @@ test('constructor: takes a default_radius argument', function(assert) {
assert.ok(osrm);
});

test('constructor: takes a default_radius unlimited argument', function(assert) {
assert.plan(1);
var osrm = new OSRM({algorithm: 'MLD', path: monaco_mld_path, default_radius: 'unlimited'});
assert.ok(osrm);
});

test('constructor: throws if default_radius is not a number', function(assert) {
assert.plan(2);
assert.throws(function() { new OSRM({algorithm: 'MLD', path: monaco_mld_path, default_radius: 'abc'}); }, /default_radius must be an integral number/, 'Does not accept string');
assert.plan(3);
assert.throws(function() { new OSRM({algorithm: 'MLD', path: monaco_mld_path, default_radius: 'abc'}); }, /default_radius must be unlimited or an integral number/, 'Does not accept invalid string');
assert.ok(new OSRM({algorithm: 'MLD', path: monaco_mld_path, default_radius: 1}), 'Does accept number');
assert.ok(new OSRM({algorithm: 'MLD', path: monaco_mld_path, default_radius: 'unlimited'}), 'Does accept unlimited');
});

test('constructor: parses custom limits', function(assert) {
Expand All @@ -135,6 +142,7 @@ test('constructor: parses custom limits', function(assert) {
max_locations_map_matching: 1,
max_results_nearest: 1,
max_alternatives: 1,
default_radius: 1
});
assert.ok(osrm);
});
Expand All @@ -150,7 +158,8 @@ test('constructor: throws on invalid custom limits', function(assert) {
max_locations_distance_table: false,
max_locations_map_matching: 'a lot',
max_results_nearest: null,
max_alternatives: '10'
max_alternatives: '10',
default_radius: '10'
})
});
});
Expand Down