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.

Sign up for GitHub

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

Adding further engine bindings #6

Merged
merged 6 commits into from
Jul 10, 2023
Merged

Adding further engine bindings #6

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2490fcb
Add further engine support
whytro Jun 30, 2023
171e825
Add __repr__ output for custom JSON types
whytro Jun 30, 2023
c5b00de
Add in MatchParameter support
whytro Jul 3, 2023
af1adc7
Add validity check for TileParameter argument
whytro Jul 3, 2023
0bc51a5
Adjust visitor for JSON types to return string
whytro Jul 3, 2023
cc5ed2b
Adjust MatchParameter arguments and handling
whytro Jul 4, 2023
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
5 changes: 5 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ set(SRCS

src/parameters/baseparameter_nb.cpp
src/parameters/routeparameter_nb.cpp
src/parameters/matchparameter_nb.cpp
src/parameters/nearestparameter_nb.cpp
src/parameters/tableparameter_nb.cpp
src/parameters/tileparameter_nb.cpp
src/parameters/tripparameter_nb.cpp

src/types/optional_nb.cpp
src/types/status_nb.cpp
Expand Down
8 changes: 8 additions & 0 deletions include/parameters/matchparameter_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef OSRM_NB_MATCHPARAMETER_H
#define OSRM_NB_MATCHPARAMETER_H

#include <nanobind/nanobind.h>

void init_MatchParameters(nanobind::module_& m);

#endif //OSRM_NB_MATCHPARAMETER_H
8 changes: 8 additions & 0 deletions include/parameters/nearestparameter_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef OSRM_NB_NEARESTPARAMETER_H
#define OSRM_NB_NEARESTPARAMETER_H

#include <nanobind/nanobind.h>

void init_NearestParameters(nanobind::module_& m);

#endif //OSRM_NB_NEARESTPARAMETER_H
8 changes: 8 additions & 0 deletions include/parameters/tableparameter_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef OSRM_NB_TABLEPARAMETER_H
#define OSRM_NB_TABLEPARAMETER_H

#include <nanobind/nanobind.h>

void init_TableParameters(nanobind::module_& m);

#endif //OSRM_NB_TABLEPARAMETER_H
8 changes: 8 additions & 0 deletions include/parameters/tileparameter_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef OSRM_NB_TILEPARAMETER_H
#define OSRM_NB_TILEPARAMETER_H

#include <nanobind/nanobind.h>

void init_TileParameters(nanobind::module_& m);

#endif //OSRM_NB_TILEPARAMETER_H
8 changes: 8 additions & 0 deletions include/parameters/tripparameter_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef OSRM_NB_TRIPPARAMETER_H
#define OSRM_NB_TRIPPARAMETER_H

#include <nanobind/nanobind.h>

void init_TripParameters(nanobind::module_& m);

#endif //OSRM_NB_TRIPPARAMETER_H
107 changes: 107 additions & 0 deletions include/types/jsoncontainer_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,115 @@
#ifndef OSRM_NB_JSONCONTAINER_H
#define OSRM_NB_JSONCONTAINER_H

#include "util/json_container.hpp"

#include <nanobind/nanobind.h>

#include <string>

void init_JSONContainer(nanobind::module_& m);

namespace json = osrm::util::json;
using JSONValue = mapbox::util::variant<json::String,
json::Number,
mapbox::util::recursive_wrapper<json::Object>,
mapbox::util::recursive_wrapper<json::Array>,
json::True,
json::False,
json::Null>;
//Custom Type Casters
namespace nanobind::detail {

template <> struct type_caster<JSONValue> : type_caster_base<JSONValue> {
template <typename T> using Caster = make_caster<detail::intrinsic_t<T>>;

template <typename T>
static handle from_cpp(T&& val, rv_policy policy, cleanup_list* cleanup) noexcept {
return mapbox::util::apply_visitor([&](auto &&v) {
return Caster<decltype(v)>::from_cpp(std::forward<decltype(v)>(v), policy, cleanup);
}, std::forward<T>(val));
}
};

template <> struct type_caster<json::String> : type_caster_base<json::String> {
static handle from_cpp(const json::String& val, rv_policy, cleanup_list*) noexcept {
return PyUnicode_FromStringAndSize(val.value.c_str(), val.value.size());
}
};
template <> struct type_caster<json::Number> : type_caster_base<json::Number> {
static handle from_cpp(const json::Number& val, rv_policy, cleanup_list*) noexcept {
return PyFloat_FromDouble((double)val.value);
}
};

template <> struct type_caster<json::True> : type_caster_base<json::True> {
static handle from_cpp(const json::True&, rv_policy, cleanup_list*) noexcept {
return handle(Py_True).inc_ref();
}
};
template <> struct type_caster<json::False> : type_caster_base<json::False> {
static handle from_cpp(const json::False&, rv_policy, cleanup_list*) noexcept {
return handle(Py_False).inc_ref();
}
};
template <> struct type_caster<json::Null> : type_caster_base<json::Null> {
static handle from_cpp(const json::Null&, rv_policy, cleanup_list*) noexcept {
return none().release();
}
};

} //nanobind::detail

struct ValueStringifyVisitor {
ValueStringifyVisitor(std::string& output) : _output(output) {};
std::string& _output;
nilsnolde marked this conversation as resolved.
Show resolved Hide resolved

void operator()(const json::String& str) {
nilsnolde marked this conversation as resolved.
Show resolved Hide resolved
_output += "'" + str.value + "'";
}
void operator()(const json::Number& num) {
_output += std::to_string(num.value);
}
void operator()(const json::True& str) {
_output += "True";
}
void operator()(const json::False&) {
_output += "False";
}
void operator()(const json::Null&) {
_output += "None";
}

void visitarray(const json::Array& arr) {
_output += "[";
for(int i = 0; i < arr.values.size(); ++i) {
if(i != 0) {
_output += ", ";
}
mapbox::util::apply_visitor(*this, arr.values[i]);
}
_output += "]";
}
void operator()(const mapbox::util::recursive_wrapper<json::Array>& arr) {
visitarray(arr.get());
}

void visitobject(const json::Object& obj) {
_output += "{";
bool first = true;
for(auto itr : obj.values) {
if(!first) {
_output += ", ";
}
_output += "'" + itr.first + "': ";
mapbox::util::apply_visitor(*this, itr.second);
first = false;
}
_output += "}";
}
void operator()(const mapbox::util::recursive_wrapper<json::Object>& obj) {
visitobject(obj);
nilsnolde marked this conversation as resolved.
Show resolved Hide resolved
}
};

#endif //OSRM_NB_JSONCONTAINER_H
32 changes: 31 additions & 1 deletion src/osrm_nb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
#include "osrm/osrm.hpp"
#include "osrm/engine_config.hpp"
#include "osrm/match_parameters.hpp"
#include "osrm/nearest_parameters.hpp"
#include "osrm/route_parameters.hpp"
#include "osrm/table_parameters.hpp"
#include "osrm/tile_parameters.hpp"
#include "osrm/trip_parameters.hpp"

#include <nanobind/nanobind.h>
#include <nanobind/stl/string.h>
Expand All @@ -14,7 +19,12 @@
#include "types/optional_nb.h"
#include "types/status_nb.h"
#include "parameters/baseparameter_nb.h"
#include "parameters/matchparameter_nb.h"
#include "parameters/nearestparameter_nb.h"
#include "parameters/routeparameter_nb.h"
#include "parameters/tableparameter_nb.h"
#include "parameters/tileparameter_nb.h"
#include "parameters/tripparameter_nb.h"

namespace nb = nanobind;

Expand All @@ -24,7 +34,12 @@ NB_MODULE(osrm_ext, m) {

using osrm::OSRM;
using osrm::engine::EngineConfig;
using osrm::engine::api::MatchParameters;
using osrm::engine::api::NearestParameters;
using osrm::engine::api::RouteParameters;
using osrm::engine::api::TableParameters;
using osrm::engine::api::TileParameters;
using osrm::engine::api::TripParameters;

init_EngineConfig(m);

Expand All @@ -36,7 +51,12 @@ NB_MODULE(osrm_ext, m) {
init_Status(m);

init_BaseParameters(m);
init_NearestParameters(m);
init_TableParameters(m);
init_RouteParameters(m);
init_MatchParameters(m);
init_TripParameters(m);
init_TileParameters(m);

nb::class_<OSRM>(m, "OSRM", nb::is_final())
.def(nb::init<EngineConfig&>())
Expand All @@ -52,6 +72,16 @@ NB_MODULE(osrm_ext, m) {

new (t) OSRM(config);
})
.def("Match", nb::overload_cast<const MatchParameters&, json::Object&>(&OSRM::Match, nb::const_))
.def("Match", nb::overload_cast<const MatchParameters&, api::ResultT&>(&OSRM::Match, nb::const_))
.def("Nearest", nb::overload_cast<const NearestParameters&, json::Object&>(&OSRM::Nearest, nb::const_))
.def("Nearest", nb::overload_cast<const NearestParameters&, api::ResultT&>(&OSRM::Nearest, nb::const_))
.def("Route", nb::overload_cast<const RouteParameters&, json::Object&>(&OSRM::Route, nb::const_))
.def("Route", nb::overload_cast<const RouteParameters&, api::ResultT&>(&OSRM::Route, nb::const_));
.def("Route", nb::overload_cast<const RouteParameters&, api::ResultT&>(&OSRM::Route, nb::const_))
.def("Table", nb::overload_cast<const TableParameters&, json::Object&>(&OSRM::Table, nb::const_))
.def("Table", nb::overload_cast<const TableParameters&, api::ResultT&>(&OSRM::Table, nb::const_))
.def("Tile", nb::overload_cast<const TileParameters&, std::string&>(&OSRM::Tile, nb::const_))
.def("Tile", nb::overload_cast<const TileParameters&, api::ResultT&>(&OSRM::Tile, nb::const_))
.def("Trip", nb::overload_cast<const TripParameters&, json::Object&>(&OSRM::Trip, nb::const_))
.def("Trip", nb::overload_cast<const TripParameters&, api::ResultT&>(&OSRM::Trip, nb::const_));
}
83 changes: 83 additions & 0 deletions src/parameters/matchparameter_nb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#include "parameters/matchparameter_nb.h"

#include "engine/api/match_parameters.hpp"

#include <nanobind/nanobind.h>
#include <nanobind/stl/string.h>
#include <nanobind/stl/vector.h>

namespace nb = nanobind;
using namespace nb::literals;

void init_MatchParameters(nb::module_& m) {
using osrm::engine::api::BaseParameters;
using osrm::engine::api::RouteParameters;
using osrm::engine::api::MatchParameters;

nb::enum_<MatchParameters::GapsType>(m, "MatchGapsType")
.value("Split", MatchParameters::GapsType::Split)
.value("Ignore", MatchParameters::GapsType::Ignore)
.export_values();

nb::class_<MatchParameters, RouteParameters>(m, "MatchParameters")
.def(nb::init<>())
.def("__init__", [](MatchParameters* t,
std::vector<unsigned> timestamps,
MatchParameters::GapsType gaps,
bool tidy,
std::vector<std::size_t> waypoints,
const bool steps,
const bool alternatives,
const RouteParameters::AnnotationsType annotations,
const RouteParameters::GeometriesType geometries,
const RouteParameters::OverviewType overview,
const boost::optional<bool> continue_straight,
std::vector<osrm::util::Coordinate> coordinates,
std::vector<boost::optional<osrm::engine::Hint>> hints,
std::vector<boost::optional<double>> radiuses,
std::vector<boost::optional<osrm::engine::Bearing>> bearings,
std::vector<boost::optional<osrm::engine::Approach>> approaches,
bool generate_hints,
std::vector<std::string> exclude,
const BaseParameters::SnappingType snapping
) {
new (t) MatchParameters(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've done the MatchParameters in the "assigning" way, but I'm not sure if it would be clearer to just fully assign all values manually, or use the constructor for MatchParameters and RouteParameters, and then manually assign for BaseParameters.

I believe the NodeJS bindings do it in the inverse way instead (ie. assigning BaseParameters, and then everything else is explicitly assigned), but I think that would be more complicated to replicate here.

Copy link
Owner

Choose a reason for hiding this comment

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

but I'm not sure if it would be clearer to just fully assign all values manually

Yeah I agree here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To clarify, do you mean that you agree with manually assigning all values over "hybrid" instantiation?

Copy link
Owner

Choose a reason for hiding this comment

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

you agree with manually assigning all values

exactly that:)

Copy link
Owner

Choose a reason for hiding this comment

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

you want to integrate that change before merging or make an issue out of it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The part that I didn't fully understand about the NodeJS annotations_type part there is that it seems to check what enum type was sent for annotations to assign the annotations_type value (in an array), but for the C++ side of things, it seems to base it off which RouteParameters constructor is called. If I'm understanding this part correctly, the user is able to send in an array of AnnotationsTypes, which gets calculated up? In that case, it would make sense to apply that same abstraction to the Python bindings.

Not sure I fully understand the question... What happens in NodeJS bindings it simply converts something like this on JS side ["nodes", "distance", "speed"] to smth like this on C++ side std::vector<AnnotationType>{AnnotationType::Nodes, AnnotationType::Distance, AnnotationType::Speed}. Can we do the same on Python side? Ofc we can use https://docs.python.org/3/library/enum.html or smth like this if needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The part that I didn't fully understand about the NodeJS annotations_type part there is that it seems to check what enum type was sent for annotations to assign the annotations_type value (in an array), but for the C++ side of things, it seems to base it off which RouteParameters constructor is called. If I'm understanding this part correctly, the user is able to send in an array of AnnotationsTypes, which gets calculated up? In that case, it would make sense to apply that same abstraction to the Python bindings.

Not sure I fully understand the question... What happens in NodeJS bindings it simply converts something like this on JS side ["nodes", "distance", "speed"] to smth like this on C++ side std::vector<AnnotationType>{AnnotationType::Nodes, AnnotationType::Distance, AnnotationType::Speed}. Can we do the same on Python side? Ofc we can use https://docs.python.org/3/library/enum.html or smth like this if needed.

It's definitely possible, but my confusion stemmed from the C++ side of things not accepting a vector of AnnotationsType - only an AnnotationsType enum. On the C++ end, is the user expected to perform that calculation prior to passing it in? Otherwise I can create an abstraction for that, like on the NodeJS bindings

Copy link
Collaborator

Choose a reason for hiding this comment

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

ahhh, I got it. annotations_type is not vector indeed, it is bitmask. Then you indeed can follow NodeJS path and just transform Python's list to this bitmask just like NodeJS bindings do.

So I would rephrase what I said above:

Not sure I fully understand the question... What happens in NodeJS bindings it simply converts something like this on JS side ["nodes", "distance", "speed"] to smth like this on C++ side AnnotationType::Nodes | AnnotationType::Distance | AnnotationType::Speed. Can we do the same on Python side? Ofc we can use https://docs.python.org/3/library/enum.html or smth like this if needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, that sounds doable, thanks for the clarifications!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've made adjustments to accommodate the requested changes. In the process, I've also discovered a bug in osrm-backend's RouteParameter's AnnotationsType |= operator overload that renders it nonfunctional, so I've also put in a PR over there to address that.

timestamps, gaps, tidy,
steps, alternatives, annotations,
geometries, overview, continue_straight
);

t->waypoints = std::move(waypoints);
t->coordinates = std::move(coordinates);
t->hints = std::move(hints);
t->radiuses = std::move(radiuses);
t->bearings = std::move(bearings);
t->approaches = std::move(approaches);
t->generate_hints = generate_hints;
t->exclude = std::move(exclude);
t->snapping = snapping;
},
"timestamps"_a,
"gaps"_a = MatchParameters::GapsType::Split,
"tidy"_a = false,
"waypoints"_a = std::vector<std::size_t>(),
"steps"_a,
"alternatives"_a,
"annotations"_a = RouteParameters::AnnotationsType::None,
"geometries"_a,
"overview"_a,
"continue_straight"_a,
"coordinates"_a = std::vector<osrm::util::Coordinate>(),
"hints"_a = std::vector<boost::optional<osrm::engine::Hint>>(),
"radiuses"_a = std::vector<boost::optional<double>>(),
"bearings"_a = std::vector<boost::optional<osrm::engine::Bearing>>(),
"approaches"_a = std::vector<boost::optional<osrm::engine::Approach>>(),
"generate_hints"_a = true,
"exclude"_a = std::vector<std::string>(),
"snapping"_a = BaseParameters::SnappingType::Default
)
.def_rw("timestamps", &MatchParameters::timestamps)
.def_rw("gaps", &MatchParameters::gaps)
.def_rw("tidy", &MatchParameters::tidy)
.def("IsValid", &MatchParameters::IsValid);
}
17 changes: 17 additions & 0 deletions src/parameters/nearestparameter_nb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#include "parameters/nearestparameter_nb.h"

#include "engine/api/nearest_parameters.hpp"

#include <nanobind/nanobind.h>

namespace nb = nanobind;

void init_NearestParameters(nb::module_& m) {
using osrm::engine::api::BaseParameters;
using osrm::engine::api::NearestParameters;

nb::class_<NearestParameters, BaseParameters>(m, "NearestParameters")
.def(nb::init<>())
.def_rw("number_of_results", &NearestParameters::number_of_results)
.def("IsValid", &NearestParameters::IsValid);
}
6 changes: 3 additions & 3 deletions src/parameters/routeparameter_nb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ void init_RouteParameters(nb::module_& m) {
using osrm::engine::api::BaseParameters;
using osrm::engine::api::RouteParameters;

nb::enum_<RouteParameters::GeometriesType>(m, "GeometriesType")
nb::enum_<RouteParameters::GeometriesType>(m, "RouteGeometriesType")
.value("Polyline", RouteParameters::GeometriesType::Polyline)
.value("Polyline6", RouteParameters::GeometriesType::Polyline6)
.value("GeoJSON", RouteParameters::GeometriesType::GeoJSON)
.export_values();

nb::enum_<RouteParameters::OverviewType>(m, "OverviewType")
nb::enum_<RouteParameters::OverviewType>(m, "RouteOverviewType")
.value("Simplified", RouteParameters::OverviewType::Simplified)
.value("Full", RouteParameters::OverviewType::Full)
.value("False", RouteParameters::OverviewType::False)
.export_values();

nb::enum_<RouteParameters::AnnotationsType>(m, "AnnotationsType", nb::is_arithmetic())
nb::enum_<RouteParameters::AnnotationsType>(m, "RouteAnnotationsType", nb::is_arithmetic())
.value("None_", RouteParameters::AnnotationsType::None)
.value("Duration", RouteParameters::AnnotationsType::Duration)
.value("Nodes", RouteParameters::AnnotationsType::Nodes)
Expand Down
Loading