movetk / movetk Goto Github PK
View Code? Open in Web Editor NEWMoveTK is a library for computational movement analysis written in C++.
Home Page: https://movetk.win.tue.nl/
License: Apache License 2.0
MoveTK is a library for computational movement analysis written in C++.
Home Page: https://movetk.win.tue.nl/
License: Apache License 2.0
When running the docker image on OSX , the documentation does not get rendered for ?movetk_core::MakePoint
.
GDAL is a heavyweight dependency for MoveTk, and a quick scan of the codebase shows that we only use it for a polygon reader and generic converters for iterators to GDAL data types. It seems to me like a user should be able to decide for themselves whether they want to use GDAL for this functionality, especially since it is heavy weight, hence we could remove the dependency and glue code. Alternatively, we could implement a kernel based on GDAL instead, but I'm not entirely sure this has a lot of added value.
We may not want to always run this, but maybe every week/month or something like that. May also be interesting to be able to manually trigger it
vcpkg
supports features
out-of-the-box, where we can give names to certain dependency sets( see this). It would be nice to use this to split up the dependencies for the different backends, as well as dependencies for building the examples/tests/documentation?
conan
can do something similar via options
, but then we have to convert the conanfile.txt
to a conanfile.py
file and implement it( see this)
The implementation of algorithms in movetk/algo/Statistics.h
assumes that the input trajectory is of type TabularTrajectory
or ColumnarTrajectory
.
However, sometimes the trajectory type may not necessarily be of these two types, for example the return type of Splitter::begin()
So maybe its better to use ``TrajectoryTraitsto infer the type of a particular field instead of using
typename Trajectory::template FieldType```
In our kernels, we name everything prefixed with Movetk
e.g. MovetkPoint
, MovetkLine
. This prefix, however, does not really add any value. Would it make sense to simply rename them to Point
, Line
etc.? This makes using movetk with user defined kernels perhaps easier, since they do not need to use the prefix.
In addition, it may be convenient to add aliases for the geometric objects in the movetk::geom
namespace, e.g.
template<typename Kernel>
using Point = typename Kernel::Point; //or MovetkPoint, if we decide not to remove the prefix
This makes using these types easier, then you can do movetk::geom::Point<Kernel>
instead of typename Kernel::Point
. This is similar to how CGAL approaches geometric objects.
The docker file in docker/with_jupyterlab
uses the cmake installed from the package manager of fedora to build MoveTK
On the other hand xeus-cling
required for running the C++ tutorials in a Jupyter notebook is installed with Miniconda.
This can be confusing. Its better to build MoveTK with cmake and dependencies installed from Miniconda
When running the tests in Debug mode on Windows, the assertion at line 285 of movetk/algo/Interpolation.h
is triggered:
assert(eps * eps < MOVETK_EPS);
This happens during test case 'kinematic trajectory interpolation 1'. eps
is a MovetkVector with coordinates (6.4709688360932933, 5.1473812288972436) for me, which evidently is much larger than MOVETK_EPS
when "squared".
I'm not sure whether the assertion is incorrect or the code does something interesting. @aniketmitra001 do you know what causes this issue?
The current version we use does not build anymore on modern Linux due to changes in libstdc++. This is fixed upstream in Catch2, but requires us bumping the version. There are some breaking changes between v2.x and v3.x though, so we need to check what needs to change.
The build fails on windows.
MoveTK is a geometry backend agnostic library. This means that the choice of the dimension, coordinate type and geometry library (Boost.Geometry or CGAL) is delegated to the application developer. Since, in the library these types are template parameters, the compiler expects these types to be defined while building the project. While writing python bindings with PyBind11, this means that the types have to be defined in the .cpp file. Therefore in the binding code , we will need to define, BoostGeometryKernel2D, BoostGeometryKernel3D, CGALGeometryKernel2D,... So the user if the python module will be restricted by the choice of the dimension and geometry backend defined in the binding code. Moreover we can limit the number type to long double. So the trade off with ease of use seems to come at the cost of lack of flexibility. We need to check if there can be a workaround to this
There is some existing code that is commented out, Usually unused typedefs and cout calls. For code quality, I suggest these be removed.
The FreespaceDiagram
defined in StrongFrechetDistance
should perhaps reside in the movetk/ds
folder, next to the other, non-parameterized, FSD. In addition, there is some duplication that can be removed with some refactoring.
GA automatically deletes the cache if it hasn't been used for a week. Especially for the dependencies, this makes the build times of the CI increase, while our dependencies do not often change. See if we can construct a script that runs once a week to keep this cache alive.
Reported by @bacusters-tue
At present there are no concept-like definitions for the geometric primitives for the kernel
Right now the problem is lets say we are using the Boost Backend and in the wrapper for boost we have added an interface for adding two points , but for the the CGAL backend we haven’t , so when we parameterise an algorithm that adds two points and use the CGAL backend , we will get a compile time error. So if we define a set of requirements on the MovetkGeometryKernel (/src/include/movetk/geom/GeometryInterface.h) wherein for instance WrapperGeometryKernel::Wrapper_Point should satisfy those requirements, we will in a way be enforcing the homogeneity of the implementation in WrapperGeometryKernel.
Would it make sense to construct a geographic/geodetic kernel, where we also support conversion between coordinate systems? We could encapsulate the coordinate transformations there instead of having separate functions that either wrap GeographicLib or perform somewhat simplified conversions. In addition, some algorithms may also work for non-Euclidean space.
The following line of code may assert unnecessarily:
Is it needed? What is it asserting to be true?
To avoid possible clashes with other libraries, I think it would be nice if we prefix MoveTk-specific CMake variables/ preprocessor variables with a common suffix such as MOVETK_. For me as a CMake gui user, this has the additional benefit that all MoveTk related CMake variables would be grouped.
Currently, this does not work (see https://movetk.github.io/movetk/)
Also investigate if we can use it for MSVC (seems to be support).
The workflow for creation of docker image fails with the following error
/opt/conda/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: warning: libwebp.so.7, needed by /opt/conda/lib/libgdal.so, not found (try using -rpath or -rpath-link)```
With the TrajectoryReader we can read trajectories from probes, possibly removing duplicate probes by for example the dates. Should we use a stable sorting algorithm before that to make sure the outcome is the same if we use the same probe data file? We now use std::sort, which does not guarantee this. We maybe could use std::stable_sort instead?
Create modules with diverse dependencies and a bare bone code base for systems missing larger libraries
This way we can enforce some standard on code styling and formatting, and makes it easier for users to contribute. We should decide what kind of code style we want for this.
make fails with the following error
[ 98%] Building CXX object py/CMakeFiles/movetk_geometry.dir/movetk_geometry.cpp.o
/home/runner/work/movetk/movetk/py/movetk_geometry.cpp:44:16: error: expected constructor, destructor, or type conversion before ‘(’ token
PYBIND11_MODULE(movetk_geometry, m) {
MEOS is a C++ library which makes it easy to work with temporal and spatiotemporal data . It is based on MobilityDB's data types and functions.
Add support for the trajectory objects of MobilityDB using MEOS's C++ API
The current implementation of Statistics.h does not seem to be able to handle degenerate cases , for instance when the trajectory size is 1.
For input data where trajectory has only one point, this results in a segmentation fault.
I think its better to handle this..
In the development meeting of October 2nd,2020 the following was suggested:
Only support this if a sensible conversion is possible. This may make it easier to support some algorithms that are easier implementable (or implementable at all) in another kernel.
The examples/include/GeometryBackendTraits.h
header file is now supposed to be included in py/movetk_geometry.cpp
. Also tutorials/TrivialGeometry.h
is similar to examples/include/GeometryBackendTraits.h
So have one copy of GeometryBackendTraits.h
and place it under src/include/movetk/utils
While you are at it move Geolife.h
, GeolifeProbeTraits.h
,GeolifeTrajectoryTraits.h
, HereProbeTraits.h
, Algorithms.h
to src/include/movetk/utils
as these are specialisations of various traits classes in the library and are mainly a collection of utility headers for the application developer
Maybe this is more of a stylistic question: currently, our namespaces are all prefixed by movetk_ and do not map directly to the folders that the associated methods/classes are in. I personally prefer to synchronize the names of the folders of the (includable) header files with the namespaces that the objects reside in. Using this guideline, it is easier to see what you need to include from a fully qualified name, e.g. movetk::core::MakePoint<> would then reside in some file in 'movetk/core' in the header include folder. Is this something we would want to pursue?
Create Python bindings for MoveTK
Currently, the CI documentation build process (in the docs folder) causes problems with merging. Need to figure out a different process.
Hi Bram
wrt the the following changes in Distances.h
//Use std::function, otherwise boost will error when trying to copy the transform_iterator
std::function<NT(const typename Kernel::MovetkPoint &)> transform = [poly_a](const auto &polyBEl) {
SqDistance sqDist;
return std::sqrt(sqDist(*poly_a, polyBEl));
};
auto maxElIt = std::max_element(boost::make_transform_iterator(poly_b, transform), boost::make_transform_iterator(poly_b_beyond, transform));
This fails to compile on some versions of clang as std::max_element
expects an iterators that is a model of a ForwardIterator
and boost::make_transform_iterator
seems to model an InputIterator
Please see this stack overflow discussion.
As suggested in the discussion of SO, we could return by reference from the lambda function but doesn't seem to be advisable. Please see the notes here
For the lack of a better alternative I have had to substitute the above with the following suboptimal piece of code
std::vector<NT> distances;
std::transform(poly_b, poly_b_beyond, std::back_insert_iterator(distances),
[poly_a](const auto &polyBEl) {
SqDistance sqDist;
return std::sqrt(sqDist(*poly_a, polyBEl));
});
auto maxElIt = std::max_element(std::begin(distances), std::end(distances));
in this commit d0e8b27
Currently, we are not running the tests as part of CI, which would help catching errors earlier.
Currently, in our Windows CI, we pull in the entirety of boost, which makes the current Windows CI process quite slow (80+ minutes). I think if we use a more finegrained selection of the boost packages, this should reduce the depedencies download and build time in CI
Currently, Distances.h is directly included in the GeometryInterface.h header. This disallows any algorithms in Distances.h from reusing code such as the MakeSth objects in GeometryInterface.h. Decoupling this dependency would be beneficial to achieve more code reuse I think.
@aniketmitra001 already made a design for decoupling
Vanilla Fedora image does not build.
Python dependency is not part of image.
After skipping this, radpidjson header files are then missing
Hi Bram
I see that in your latest commits you have disabled the setting of the first value on the output iterator for the above functions
//*iter = 0;
with the following comment
Add a less strict input iterator requirement. Add point iterators to trajectory utils and make some function a bit less strict on the iterators
The idea of setting the first value to zero was to ensure that the size of the output is same as the input, as as we treat the result of movetk_core::get_time_diffs()
as a column that can be added to the tabular trajectory data structure , so if the tabular trajectory has size m x n, then the output of movetk_core::get_time_diffs()
is m x 1
Not setting the first value to 0 results in an output of size (m - 1) x 1
For N timestamps, get_time_diffs returns n-1 results. Should this be N, with the first value being 0?
For N positions, get_distances returns N results. Neither the first or last distance is 0. One of these should be incorrect.
Both methods should be consistent.
Calculation appears to be difference between N and N +1, hence It appears that the last distance calculation must be incorrect as there is no N + 1
However, a change in behaviour will break existing code, so more clarification in the API documentation is perhaps called for.
The current GeoJSONUtil.h
in movetk/io/
creates the GeoJSON structure by adding to a stream operator and is not very extendable.
It should be ProbeParseDate instead of std::string
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.