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

Floating point rounding error when writing c3d #264

Closed
felixchenier opened this issue Aug 5, 2022 · 2 comments
Closed

Floating point rounding error when writing c3d #264

felixchenier opened this issue Aug 5, 2022 · 2 comments

Comments

@felixchenier
Copy link
Contributor

Salut Benjamin

In ezc3d/init.py, lines 481 to 487:

                if (
                    nb_analog_frames
                    != self._storage["parameters"]["ANALOG"]["RATE"]["value"][0]
                    / self._storage["parameters"]["POINT"]["RATE"]["value"][0]
                    * nb_point_frames
                ):
                    raise ValueError("Number of frames in the data set must match the analog rate X point frame")

I think there should be accommodation for floating-point rounding error before raising the ValueError. I have some data where nb_analog_frames = 4420, but the calculation of

self._storage["parameters"]["ANALOG"]["RATE"]["value"][0]
                    / self._storage["parameters"]["POINT"]["RATE"]["value"][0]
                    * nb_point_frames

gives 4420.000000000001.

So I get a ValueError instead of successfully saving the c3d. In my view this is only a floating-point difference and the function should go on with saving anyway. I could modify it myself and do a PR, but I'm not sure, is this code autogenerated or it's manually maintained?

Thanks

@felixchenier
Copy link
Contributor Author

In any case, changing these lines to

                if nb_analog_frames != round(
                    self._storage["parameters"]["ANALOG"]["RATE"]["value"][0]
                    / self._storage["parameters"]["POINT"]["RATE"]["value"][0]
                    * nb_point_frames
                ):

solves the problem.

@pariterre
Copy link
Member

Hi @felixchenier
The code is manually generated so changing it makes senses :)
I think you can PR this modification :)

Thanks!

felixchenier added a commit to felixchenier/ezc3d that referenced this issue Aug 5, 2022
pariterre pushed a commit that referenced this issue Aug 8, 2022
felixchenier referenced this issue Jan 19, 2023
I feel that this method is a smoother alternative. 
This will be able to perform the check without any errors caused and would not rely on other functions to smooth the value out.
felixchenier added a commit to felixchenier/ezc3d that referenced this issue Jan 19, 2023
This re-solves pyomeca#264 which had been cancelled by commit ba50ee5
pariterre added a commit that referenced this issue Jan 20, 2023
felixchenier added a commit to kineticstoolkit/kineticstoolkit that referenced this issue Mar 9, 2023
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

2 participants