salmagro commented 5 months ago

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

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 commented 5 months ago

can you open a PR, please?