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

Use consistent float / double precision by using configuring the ostream object precision? #790

Closed
EricCousineau-TRI opened this issue Dec 17, 2021 · 5 comments · Fixed by #872
Assignees
Labels
enhancement New feature or request

Comments

@EricCousineau-TRI
Copy link
Collaborator

EricCousineau-TRI commented Dec 17, 2021

Desired behavior

Ensure all objects emit floating point numbers of same precision.

At present, there may be different (default?) precisions in use by composite types in ign::math, as encountered by:
#689 (comment)

Alternatives considered

N/A

Implementation suggestion

Consider setting precision in ostream object inside sdformat, which is then (hopefully) used directly by composite types.

Additional context

N/A

\cc @azeey

@jennuine
Copy link
Collaborator

Originally was targeting sdf12 but now will be targeting sdf13 (ie. Ign Garden). The approach is to update all float/double string outputs to be consistent and upon further investigation this will require minor behavior changes in (1) SDFormat and (2) ign-math:

(1) Depending on the condition for poses, the original string may be printed regardless of the precision setting. To make things consistent this will need to be changed so the default precision (6 digits) is used unless the max precision is specified. For ex,

input: <pose>1 1.0 1.5707963267948966 3.14159265358979323846 0 1.57079632679489667</pose>
current output: 1 1.0 1.5707963267948966 3.14159265358979323846 0 1.57079632679489667 
// note above output uses _originalStr so y=1.0 instead of just 1

new default output: 1 1 1.5708 3.14159 0 1.5708
new max output: 1 1 1.5707963267948968 3.1415926535897931 0 1.5707963267948968

(2) There is an issue with changing the precision for ignition::math::Vector3 because of this precision function being used. Essentially, ign-math changes the precision to 6 and when it goes through libsdformat it's no longer the full number (it's the 6 digit number)

math::Vector3d test(1.5707963267948966, 0, 0)
cout << test; // prints 1.5708 -0 0
cout << setprecision(max) << test; // 1.5707960000000001 -0 0; x should be 1.5707963267948968

@azeey
Copy link
Collaborator

azeey commented Feb 3, 2022

  1. If we're changing behavior and putting this in sdf13, it might be better to make the default use max precision. This will likely make the output more verbose, but IMO, preserving the precision, especially on poses, is more important.

  2. I'm in favor of changing the behavior of Vector3 so that it doesn't change the precision at all when printing values.

/cc @aaronchongth , @EricCousineau-TRI

@aaronchongth
Copy link
Collaborator

Thanks for bringing this up!

  1. I agree with @azeey, and luckily all the logic for printing poses should be well contained in these few lines, so all other types can have consistent precision implemented without affecting it.

  2. Makes sense! Will this only be applied to Vector3? If so, we can probably parse it accordingly under ParamPrivate::StringFromValueImpl

@jennuine
Copy link
Collaborator

jennuine commented Feb 9, 2022

Will this only be applied to Vector3?

The changes are applied to Vector2/3/4, Color, and Matrix3/4 (ign-math targeting garden).


Here are the PRs related to (2)

@jennuine jennuine linked a pull request Mar 9, 2022 that will close this issue
8 tasks
@azeey
Copy link
Collaborator

azeey commented Apr 18, 2022

Closed by #872.

@azeey azeey closed this as completed Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants