opensim-org / opensim-core

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

A strange behavior when appending a column (VectorView) to a table #3654

Open mrrezaie opened 9 months ago

mrrezaie commented 9 months ago

Hi, I found a strange behavior when I'm trying to append a new VectorView column to a TimeSeriesTable; but it's hard to explain. To reproduce and simplify it, please see this example:

import opensim as osim
import matplotlib.pyplot as plt
IKName    = 'input/inverse_kinematics.mot'

# part one
q = osim.TimeSeriesTable(IKName)
vector_view = q.getDependentColumn('knee_angle_r')
q.appendColumn('new', vector_view)
q.addTableMetaDataString('nColumns', str(q.getNumColumns()))

# part two
plt.plot(vector_view.to_numpy(), label='original')
plt.plot(q.getDependentColumn('new').to_numpy(), ls='--', label='appended')
plt.title('knee_angle_r')
plt.legend()
plt.show(block=False)

Here, I read a coordinates file (output of IK tool) and take values of a coordinate (knee angle) and append the values to the very table as a new column. Notice parts one and two in the comments. When I run the entire script in my console, everything is correct and both new and original values match. But, if I run parts separately, the original values would be changed. Here is a video of it:

https://github.com/opensim-org/opensim-core/assets/38867713/9b0079fe-5f9b-48df-8a8b-bbcba0dadd17

Any idea why this is happening?

There is a workaround; if I append the column as a Vector (not VectorView), there would be no issue at all:

vector_numpy = q.getDependentColumn('knee_angle_r').to_numpy()
q.appendColumn('new', osim.Vector(vector_numpy))

Windows 10, Python 3.11, OpenSim 4.5

Thank you in advance.

nickbianco commented 9 months ago

@mrrezaie I'm not super familiar with how workspace variables are saved in Jupyter notebooks, but since a VectorView is inherently a reference to a Vector quanitity, it could be that the reference gets cleared when the two snippets are run separately, whereas a Vector gets stored in memory in the workspace that persists across snippets.

So this might not be a bug, but rather the nature of VectorView vs Vector.

mrrezaie commented 9 months ago

Hi @nickbianco Thanks for your response. Your explanations make sense to me; however, this was an IPython console not a Jupyter notebook. The output is so unstable and sometimes it works fine. I found something new; please see this new example which always happens:

q = osim.TimeSeriesTable(IKName)
vector_view  = q.getDependentColumn('knee_angle_r')
vector_numpy = q.getDependentColumn('knee_angle_r').to_numpy()
q.removeColumn('knee_angle_r')

plt.plot(vector_view.to_numpy(), label='VectorView')
plt.plot(vector_numpy, ls='--', label='Numpy')

image

  1. It seems that some sort of connection remains between the VectorView (obtained by getDependentColumn) and the TimeSeriesTable, i.e., its values may change.

  2. Please note that the q table was in Degrees. In this example, if I convert the vector_numpy to Radians, they will match:

    plt.plot(vector_view.to_numpy(), label='VectorView')
    plt.plot(vector_numpy/(180/osim.SimTK_PI), ls='--', label='Numpy')

    image

Isn't it strange?

nickbianco commented 9 months ago

Whether Jupyter or IPython console, my comment about workspace management still stands. I'd recommend trying to debug the issue at a more granular level. If you run part one of your original script, can you access the VectorView immediately after without issue? Do the contents that the VectorView point to change after running another command in the console?

To go one step further, you could try testing the behavior in a different scripting environment (e.g., Matlab) to test if this is something specific to the IPython console, or if this behavior is more general to scripting with VectorView types.

mrrezaie commented 9 months ago

Thank you @nickbianco, but I have no access to MATLAB.

I could reproduce the latest script in Google Colab, just to make sure that nothing is wrong with my Python environment.

The first impression of VectorView seems to be stable. Any later call to the table (q), e.g., appendColumn or removeColumn may change its values.

Please let me know if there is anything that I can test.

nickbianco commented 9 months ago

In Python, get a column from a table might actually make a copy even though it is a VectorView type, but I'm not totally sure off the top of my head.

It is strange that removing the column from the table above led to the vector printing in radians and not degrees. If a copy was made, I don't see how the units could change. I'm actually not sure how the units could change at all given the snippet above.

If you do anymore testing, I would pull on that thread a bit more: why are the units changing? For what types does this unit change happen (e.g., only VectorView? or also Vector?).

mrrezaie commented 9 months ago

I did more tests using this file \OpenSim-4.5-2023-12-04-win64\Resources.zip\Code\Python\Moco\example3DWalking\coordinates.sto and this snippets:

column1 = '/jointset/walker_knee_r/knee_angle_r/value'
column2 = '/jointset/hip_r/hip_rotation_r/value'
column3 = '/jointset/ankle_r/ankle_angle_r/value'

for test in range(50):

    q = osim.TimeSeriesTable('coordinates.sto')
    vector_view  = q.getDependentColumn(column1)
    values1 = [vector_view[i] for i in range(vector_view.size())]

    q.removeColumn(column1)
    values2 = [vector_view[i] for i in range(vector_view.size())]

    q.removeColumn(column2)
    values3 = [vector_view[i] for i in range(vector_view.size())]

    q.removeColumn(column3)
    values4 = [vector_view[i] for i in range(vector_view.size())]

    plt.figure()
    plt.plot(values1, label='values1')
    plt.plot(values2, label='values2', ls='--')
    plt.plot(values3, label='values3', ls='dotted')
    plt.plot(values4, label='values4', ls='-.')
    plt.title(f'Test #{test}')
    plt.legend()
    plt.show(block=False)

    del q, vector_view

This is the output:

https://github.com/opensim-org/opensim-core/assets/38867713/811ebfd2-e526-4a2c-a5b0-a1ff7514301f

It seems that there is a link between the VectorView and the table. Whenever I call removeColumn, something is changing randomly.

(I don't know how to test/get a Vector from the table)

nickbianco commented 9 months ago

@mrrezaie, this isn't too surprising to me. When you call removeColumn, the table is probably just releasing its ownership of the data stored in that column. If you then query the VectorView, which is just a pointer to some data, you will get something back, but there's no guarantee that this view is still pointing to the same data.

In general, it's probably a bad idea to get a view into the data, then modify the table, and then try to use the data view. In the case of IPython/Jupyter, it might be a bad idea to expect a vector view to persist in the local workspace across different scripts or code snippets. I still don't have a good sense if there's an OpenSim bug here or if this is just a side-effect of using VectorViews through the OpenSim bindings in IPython, but my guess is the latter.

mrrezaie commented 9 months ago

@nickbianco, thanks for clarification. Now, I have a better understanding of what is happening. I'm looking forward an improvement in this regard.

One more thing, I can easily convert a RowVectorView to a RowVector, but such constructor doesn't exist for Vector. In this example, converting the VectorView to Vector might be a better alternative than to_numpy.

List of RowVector constructors:

SimTK::RowVector_< double >::RowVector_()
SimTK::RowVector_< double >::RowVector_(SimTK::RowVector_< double > const &)
SimTK::RowVector_< double >::RowVector_(SimTK::RowVectorBase< double > const &)
SimTK::RowVector_< double >::RowVector_(int)
SimTK::RowVector_< double >::RowVector_(int,double const *)
SimTK::RowVector_< double >::RowVector_(int,double const &)
SimTK::RowVector_< double >::RowVector_(std::vector< double,std::allocator< double > > const &)

List of Vector constructors:

SimTK::Vector_< double >::Vector_()
SimTK::Vector_< double >::Vector_(SimTK::Vector_< double > const &)
SimTK::Vector_< double >::Vector_(int,double const &)
SimTK::Vector_< double >::Vector_(std::vector< double,std::allocator< double > > const &)