opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
758 stars 308 forks source link

An issue related to `TimeSeriesTable::trim` #3751

Closed mrrezaie closed 1 day ago

mrrezaie commented 3 months ago

Hi, I think something is wrong with trim method.

I'm using the following piece of script to apply low-pass filter on TimeSeriesTable. After removing the pads using trim, it suppresses a row from the table.

timeColumn = table.getIndependentColumn() # time
osim.TableUtilities().filterLowpass(table, 10, padData=True)
table.trim(timeColumn[0], timeColumn[-1]) # remove padding

It often works well but not always and it interrupts my workflow. Perhaps the issue is somehow related to round-off error in the time column. Is there any better and more robust way to remove the pads (in Python)?

In API, a better alternative would be the combination of getNearestRowIndexForTime and trimToIndices, but the later one has not been exposed yet for Python. I was wondering if you could expose trimToIndices, or integrate the 'search for the nearest time' with trim method.

Thank you in advance.

nickbianco commented 3 months ago

Hi @mrrezaie, the docs say:

Uses getRowIndexAfterTime to locate first row and getNearestRowIndexForTime method to locate last row.

so the trimming might not be inclusive to the first time point. You could try providing a time value just before your initial time to include the first time index.

I'm not sure we will change this functionality, but regardless we should expose trimToIndices so you have more control over the start time index.

mrrezaie commented 3 months ago

Hi @nickbianco, thanks for your response, and exposing trimToIndices.

Uses getRowIndexAfterTime to locate first row and getNearestRowIndexForTime method to locate last row. so the trimming might not be inclusive to the first time point. You could try providing a time value just before your initial time to include the first time index.

Not sure if trim is functioning as documented. In this example, it is including the first time.

import opensim as osim
import numpy as np

times = np.linspace(0,1,11)
# [0. 0.1 0.2 0.3 0.4 0.5 0.6 0.7 0.8 0.9, 1. ]
col = np.arange(0,11, dtype=float)
# [0. 1. 2. 3. 4. 5. 6. 7. 8. 9. 10.]

table = osim.TimeSeriesTable(times)
table.appendColumn('col', osim.Vector(col))

table.trim(0.1,0.9)
print(table.getDependentColumn('col').to_numpy())
# [1. 2. 3. 4. 5. 6. 7. 8. 9.]

And if some sort of rounding error occurs, the output might be unreliable:

error = 1e-12
times = np.linspace(0,1,11) - error
...
table.trim(0.1,0.9)
# [2. 3. 4. 5. 6. 7. 8. 9.]
times = np.linspace(0,1,11) + error
...
table.trim(0.1,0.9)
# [1. 2. 3. 4. 5. 6. 7. 8.]

But getNearestRowIndexForTime always returns the correct index. Thank you.

tkuchida commented 3 months ago

Not sure if trim is functioning as documented.

IMHO the behavior in your examples is correct: trimming based on time points will always be sensitive to numerical noise. The trim method guarantees that all returned points are within the specified range. If what you're trying to do is find the time point closest to a specified value, then getNearestRowIndexForTime is the right idea. As @nickbianco suggested, a trimToIndices method would be good if there's currently no way to do this. 🐴

mrrezaie commented 3 months ago

Thanks for your explanation. So, I'll be looking forward to trimToIndices method. Thanks for your time.