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
Read/write transforms in read_c3d and write_c3d #229
Comments
Hi @felixchenier, Just started working on the change. The The issue mainly lies in the assumption of the function that there must be point data in the c3d file. In files where there are rotation matrices, there is often no point data available. In the current format, that is not possible since the first check in the function is I was thinking about making if points is None and analgos is None and rotations is None:
raise ValueError("Message")
if points is not None:
# add points
if analogs is not None:
# add analogs, may need some changes since it now checks wheter the frame rate related to the points frame rate
if rotations is not None:
# add rotations
# wride c3d and add events The main function change will that there is a possibility for a c3d file with I am asking since I am probably missing something and am not sure what your intentions and ideas have been thusfar. |
Hi @Alek050 This is an interesting question. I agree that
should be changed to:
with None by default for the points. It just makes sense. Now for the implementation, I read on https://www.c-motion.com/v3dwiki/index.php?title=ROTATION_DATA_TYPE : ROTATION:RATE and ROTATION:RATIORATE = Sampling Rate RATIO = Ratio of ROTATION sample rate to POINT sample rate The C3D file format is designed to store synchronized 3D Point data, analog data, and Rotation data; Rotation measurements are recorded at fixed intervals (set by the ROTATION:RATE parameter). For consistency with C3D file prior to the introduction of the ROTATION group, the POINT:RATE is considered the base rate of a C3D file. If the ROTATION rate is stored at a higher, integer multiple ratio of the POINT:RATE, the ANALOG:RATIO parameter defines this multiple. Storing the RATE and RATIO is redundant, but for historical reasons both are stored. Therefore, we must have a POINT:RATE, even if we have no points. My idea, in case there are no points, would be to create, at the very beginning of the function, a dummy As you mention, it would also solve the hypothetical problem of having only analogs in the C3D, which I effectively overlooked. |
Thanks for you clear response. I just updated the code to also include writing files. I have not tested it elaborately but it seems like it gives the expected output. There are two issues though. First, the
I've played around a bit but the error pursues. Funny enough, the output c3d is exactly as expected and there is no problem reading in this file. Have you seen this error earlier? Secondly, I could be wrong, but I think I have opened a work in progress (WIP) pull request at #232, could you have a look at the code, provide some feedback and shine your light on the above mentioned issues? Much appreciated! PS I have yet to write the tests for the |
Hi @Alek050 Great work, really :-)
No, I don't remember to have encountered this error before. Does it happen with any file, or just when you try to write rotations?
It may become a problem if data doesn't start at 0s. I understand that In general I agree with everything you wrote. This review also allowed me to spot a potential problem in writing events: at the moment, only the events from the points TimeSeries are written. The better way to define the events would be to merge all events from all TimeSeries:
Then use |
Hi @felixchenier, Just opened an issue in ezc3d, as I suspect it is a problem from their side. Thanks for providing the code for the events, I am not working with events within c3d files for now so this is new for me, but I will update the code based on your recommendations. Regarding the error, I get it with writing any file, for instance when reading in the import kineticstoolkit as ktk
ts = ktk.read_c3d("data/kinematics_racing_full.c3d")
ktk.write_c3d("test_file", points=ts["Points"])
So I think it has something to do with my environment in combination with UPDATED: import numpy as np
import ezc3d
# Load an empty c3d structure
c3d = ezc3d.c3d()
# Adjust some mandatory values of the parameters and fill the data with random values
c3d["parameters"]["POINT"]["RATE"]["value"] = [100]
c3d["parameters"]["POINT"]["LABELS"]["value"] = ("point1", "point2", "point3", "point4", "point5")
c3d["data"]["points"] = np.random.rand(3, 5, 100)
c3d["parameters"]["ANALOG"]["RATE"]["value"] = [1000]
c3d["parameters"]["ANALOG"]["LABELS"]["value"] = ("analog1", "analog2", "analog3", "analog4", "analog5", "analog6")
c3d["data"]["analogs"] = np.random.rand(1, 6, 1000)
# Create a custom parameter to the POINT group
c3d.add_parameter("POINT", "NewParam", [1, 2, 3])
# Create a custom parameter a new group
c3d.add_parameter("NewGroup", "NewParam", ["MyParam1", "MyParam2"])
# Write a new modified C3D and read back the data
c3d.write("temporary.c3d") Similarly, the example of import kineticstoolkit.lab as ktk
import numpy as np
points = ktk.TimeSeries()
points.time = np.linspace(0, 10, 10*240, endpoint=False)
points.data["Marker1"] = np.ones((2400, 4))
points.data["Marker2"] = np.ones((2400, 4))
analogs = ktk.TimeSeries()
analogs.time = np.linspace(0, 10, 10*2400, endpoint=False)
analogs.data["Signal1"] = np.sin(analogs.time)
analogs.data["Signal2"] = np.cos(analogs.time)
rotations = ktk.TimeSeries()
rotations.time = np.linspace(0, 10, 10*480, endpoint=False)
rotations.data["Rotation1"] = np.ones((4800, 4, 4))
rotations.data["Rotation2"] = np.ones((4800, 4, 4))
ktk.write_c3d("testfile.c3d", rotations=rotations)
ktk.write_c3d("testfile.c3d", points=points)
ktk.write_c3d("testfile.c3d", analogs=analogs)
ktk.write_c3d("testfile.c3d", points=points, analogs=analogs, rotations=rotations) I am not really sure why it does not work since |
Hi @Alek050 It's weird, but then it reminded my something, an issue that I reported myself several months ago on ezc3d: Is it possible that:
I did a workaround for this issue on Windows in Thanks |
Hi @felixchenier, Thanks for looking into this, I replied in the new opened issue. I was looking into your earlier mentioned suggestion regarding the events and found some potential issues with this approach: First, merging TimeSeries only seem to work when the number of frames/ time vecotors are equal: points = ktk.TimeSeries(time=np.linspace(0, 10, 100,), data={"point1": np.random.rand(100)})
analogs = ktk.TimeSeries(time=np.linspace(0, 10, 1000), data={"force1": np.random.rand(1000)})
points.merge(analogs)
Obviously, this makes sense. We could of course simply resample the Secondly, I found in the """
Caution
-------
Attempting to resample a series of homogeneous matrices would likely
produce non-homogeneous matrices, and as a result, transforms would not
be rigid anymore. This function can't detect if you attempt to resample
series of homogeneous matrices, and therefore won't generate an
error or warning.
""" When resampling |
@Alek050 You know you rock, right? ;-) Yeah, I was thinking the same, this merging stuff is running from problems. The way better way is to do something like: points = points.copy()
if analogs is not None:
points.events.extend(analogs.events)
if rotations is not None:
points.events.extend(rotations.events)
points.remove_duplicate_events(in_place=True)
# Then add the events as in the original function Much better isn't it? |
Hi @felixchenier, Thanks for the appreciation! To be fair, it is truly insane that you have methods like I wanted to finalize the tests, but stumbled on another unexpected behaviour in the writing of ezc3d when using rotations, so I extended my issue there, lets hope it can be fixed easily so that we can finish this issue. |
Just wanted to give a quick update, I had some conversations with pariterre regarding how to change the For the kineticstoolkit a solution would be to ignore the start frame when writing files and there are rotations, but this can only be done if different systems (points, analogs) always start at the same time, which is not an assumption we can make I guess. On top of that it could create problems with backward compatability since we need to change the times of the events, points, and analogs will change based on what is expected. I think we will have to wait if things change on the ezc3d side, but let me know if you have any other ideas. |
Hi @Alek050
And I guess that documented and robust partial support is preferred to no support at all. For now, maybe we should make a check condition where as soon as there are rotations involved, time[0] must be 0s. for everything? I think it's the safest direction that we can take, since it shouldn't break, and it will allow users to write rotations (if there are, really! 😉) even if they need to offset their time. This would let us finish this issue, and then we could open a new, related improvement issue to allow writing rotations that start at other than 0s? |
Done in branch PR232 |
Hi @Alek050 A new version of ezc3d has been released today with the fix for the rotations. However, this test that you wrote in large part fails and I can't find why because it should pass. In this test, rotations have twice the sample rate of the points. When we save and load back, they are 4 times as fast as they were recorded. This problem doesn't happen when points and rotations have the same sample rate, everything works well.
I wonder if it's something in my code or in ezc3d, and since you work with a custom version of ezc3d, I'd like to know if this code works or not on your side. By the way I'm on the PR232 branch of the main kineticstoolkit repository. Tx |
As discussed here: https://github.com/orgs/kineticstoolkit/discussions/116#discussioncomment-8644282
It's possible to store full transform matrices in c3d files. These transforms are supported by ezc3d. It would be logical to be able to read/write those transforms as requested by @Alek050.
The text was updated successfully, but these errors were encountered: