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

Variable precision for String::String(const T& t) and Xml::setValueAs(const T& t) #775

Closed
fcanderson opened this issue Jan 30, 2024 · 3 comments

Comments

@fcanderson
Copy link
Contributor

@sherm1 I hope you are well! It's been a while.

I'm following up about setting the output precision in String::String(constT& t) to lossless. See #762.

Based on interacting with the OpenSim Developer Community at its monthly meetings, there is consensus that the ability to vary output precision would be desirable. Fixing the precision to lossless impacts both OpenSim model files (.osim files) and state serialization files (.ostates files), both of which use SimTK::Xml to write the files.

For model files, a precision of ~6 is preferred. Writing elements with lossless precision (20) can make the model files more challenging to read and edit. (User still manually edit model files.) In addition, situations arise where a number like 1.5 is changed to 1.499999999999999999999. Ayman made the comment that such changes make displaying numbers in the GUI an issue and file diffs become harder to interpret. Ajay made the comment that we don't want to give OpenSim users the impression that the values of model parameters are known to a precision of 20.

For state serialization, the desired precision could vary. For plotting, a precision ~6 is probably more than enough, while other applications (e.g., perturbation analysis, resuming a simulation midway through, etc.) might require lossless precision.

Would you be open to me submitting a PR that adds variable precision? I propose the following changes to the SimTK API:

String::String(const T& t) --> String::String(const T& t, int precision = 6)
Xml::Element::setValueAs(const T& value) --> Xml::Element::setValueAs(const T& value, precision = 6)

6 is the default C++ output precision. It's what users have unknowingly been using.

I would be happy to augment current unit tests and update the documentation.

Thanks,
Clay

@sherm1
Copy link
Member

sherm1 commented Jan 30, 2024

Sounds like a good plan!

@fcanderson
Copy link
Contributor Author

Great. I'll probably submit the PR in about a week. Thanks.

@fcanderson
Copy link
Contributor Author

fcanderson commented Mar 5, 2024

#776 and #779 add the capacity to specify numerical precision for conversion to SimTK::Strings and XML document Elements.

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