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

An issue related to TimeSeriesTable::trim #3751

Closed
mrrezaie opened this issue Mar 23, 2024 · 4 comments
Closed

An issue related to TimeSeriesTable::trim #3751

mrrezaie opened this issue Mar 23, 2024 · 4 comments
Assignees

Comments

@mrrezaie
Copy link
Contributor

mrrezaie commented Mar 23, 2024

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 nickbianco self-assigned this Mar 28, 2024
@nickbianco
Copy link
Member

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
Copy link
Contributor Author

mrrezaie commented Mar 29, 2024

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
Copy link
Member

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
Copy link
Contributor Author

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

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

4 participants