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

Misalignment in Time Series Data Due to Differing Trajectory Sizes with reactive scripts #968

Open
salmagro opened this issue Apr 23, 2024 · 2 comments

Comments

@salmagro
Copy link
Contributor

Issue: Misalignment in Time Series Data Due to Differing Trajectory Sizes with reactive scripts

Problem Description

Currently, PJ uses the nearest time index for data retrieval (TimeseriesRef::atTime(double t)), which can lead to inaccuracies when analyzing time series data of different sizes. This method may inadvertently use outdated data towards the end of a data set, affecting analysis reliability.

Example of the Problem

Please found some dummy data here: DummyData_2024-03-19.csv

Expected Currently
2024-04-23_09-38_trajectorycropped 2024-04-23_09-36_trajectory_error

Implemented Workaround

I have exposed the internal index through TimeseriesRef::getIndexAtTime(double t) to ensure that indices match when correlating data points across different series. This ensures that comparisons and analyses use data points from the same measurement intervals.

diff --git a/plotjuggler_base/include/PlotJuggler/reactive_function.h b/plotjuggler_base/include/PlotJuggler/reactive_function.h
index 601cfe43..0e903408 100644
--- a/plotjuggler_base/include/PlotJuggler/reactive_function.h
+++ b/plotjuggler_base/include/PlotJuggler/reactive_function.h
@@ -27,6 +27,8 @@ struct TimeseriesRef
 
   double atTime(double t) const;
 
+  int getIndexAtTime(double t) const;
+
   unsigned size() const;
 
   void clear() const;
diff --git a/plotjuggler_base/src/reactive_function.cpp b/plotjuggler_base/src/reactive_function.cpp
index d9c713a3..b15a73f0 100644
--- a/plotjuggler_base/src/reactive_function.cpp
+++ b/plotjuggler_base/src/reactive_function.cpp
@@ -114,6 +114,7 @@ void ReactiveLuaFunction::prepareLua()
   _timeseries_ref["at"] = &TimeseriesRef::at;
   _timeseries_ref["set"] = &TimeseriesRef::set;
   _timeseries_ref["atTime"] = &TimeseriesRef::atTime;
+  _timeseries_ref["getIndexAtTime"] = &TimeseriesRef::getIndexAtTime;
   _timeseries_ref["clear"] = &TimeseriesRef::clear;
 
   //---------------------------------------
@@ -187,9 +188,16 @@ void TimeseriesRef::set(unsigned index, double x, double y)
 double TimeseriesRef::atTime(double t) const
 {
   int i = _plot_data->getIndexFromX(t);
+  std::cout<<"[t, i, y] = ["<<t<<" ; "<<i<<" ; "<<_plot_data->at(i).y<<std::endl;
   return _plot_data->at(i).y;
 }
 
+
+int TimeseriesRef::getIndexAtTime(double t) const
+{
+  return _plot_data->getIndexFromX(t);
+}
+
 unsigned TimeseriesRef::size() const
 {
   return _plot_data->size();

Code Snippet Illustrating the Workaround

function CreateSeriesFromArray(new_series, prefix, suffix_X, suffix_Y, timestamp)
    new_series:clear()
    local index = 0, internal_index, internal_prev_index = 0, -1

    while true do
        local series_x = TimeseriesView.find(string.format("%s[%d]/%s", prefix, index, suffix_X))
        if not series_x then break end

        internal_index = series_x:getIndexAtTime(timestamp)
        if internal_index ~= internal_prev_index then break end

        local x = series_x:atTime(timestamp)
        local y = (TimeseriesView.find(string.format("%s[%d]/%s", prefix, index, suffix_Y))):atTime(timestamp)
        new_series:push_back(x, y)
        internal_prev_index = internal_index
        index = index + 1
    end
end

Proposed Solution

To address this issue more robustly, I propose enhancing the TimeseriesRef class to support both exact and nearest match strategies for time-based data retrieval:

enum class MatchType {
    Exact,    // Returns an index only if the exact time is found
    Nearest   // Returns the nearest time index (current behavior)
};

std::optional<unsigned> getIndexAtTime(double t, MatchType match_type) const {
    if (match_type == MatchType::Exact) {
        auto it = std::find_if(_plot_data->begin(), _plot_data->end(),
                               [t](const auto& point) { return point.x == t; });
        if (it != _plot_data->end()) {
            return std::distance(_plot_data->begin(), it);
        }
        return std::nullopt; // Exact time not found
    } else {
        return _plot_data->getIndexFromX(t); // Nearest match
    }
}

double atTime(double t, MatchType match_type) const {
    auto index = getIndexAtTime(t, match_type);
    if (!index) {
        throw std::runtime_error("Time point not found for exact match requirement");
    }
    return _plot_data->at(*index).y;
}

However, I think that @facontidavide could have better ideas.

@facontidavide
Copy link
Owner

can you open a PR, please?

@salmagro
Copy link
Contributor Author

can you open a PR, please?

Sure, please see #969

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants