AcademySoftwareFoundation
Write full precision of float to config file
As this depends on the patch to the yaml code, consider this proof of concept, rather than fully mergable (although it should work).
This avoids issues when (re)writing config files and drifting floating point numbers due to the required precision to exactly store floating point numbers.
For details see http://www2.open-std.org/JTC1/SC22/WG21/docs/papers/2005/n1822.pdf
I have reported the yaml-cpp bug upstream (https://code.google.com/p/yaml-cpp/issues/detail?id=272)
looks like upstream moved to github.... and the issue might be lost (https://github.com/jbeder/yaml-cpp/issues/272) but there is a pull request targeting a similar fix
https://github.com/jbeder/yaml-cpp/pull/470
Kevin
@KevinJW Please double-check with master because changes were recently done. Some Ops now hold double values (instead of float values) i.e. #640 for Matrix, etc.
Kevin, no need to recheck this quite yet. As Patrick wrote, the underlying MatrixOpData is now able to hold doubles and we read double precision from CLF files (and have a unit test for that). HOWEVER, the YAML load/save is still float based. So it's very likely that the issue you wrote this PR to address is still present. Please stay tuned and we'll get back to you on this.
Even if the MatrixOp values moved to double precision, the YAML read/write was not updated. A pull request will soon pop to complete the MatrixTransform transition to doubles.
Note that the real bug in the yaml library limits the correct output of float or double - this aspect relates to #690 if we bundle a broken library we'll always be limited to its output capabilities. yes switching to doubles 'fixes' the float precision issues, but from my reading of the code doubles will still break.
For reference the fix to the yaml-cpp https://github.com/jbeder/yaml-cpp/pull/649 is from December 2018 (not in a release currently), so either we bump the bundled version and deal with any fall out, and we document that older versions will have a slight chance of this occurring.
Kevin
Agree. As you mention, it should be part of the ext library refactoring needed for the ASWF transition (i.e. update all 'core' third-party libraries to their latest version).
OCIO v2 is now using the newest yaml-cpp release by default. Does that solve the issue this PR was intending to fix @KevinJW ?
