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

Expose precision control for float/double output #106

Closed
GoogleCodeExporter opened this issue Mar 30, 2015 · 22 comments
Closed

Expose precision control for float/double output #106

GoogleCodeExporter opened this issue Mar 30, 2015 · 22 comments

Comments

@GoogleCodeExporter
Copy link

Hello!

I have been using yaml-cpp in a project (OpenColorIO), and we just switched 
from svn r423 to r482.   As part of our file format, we often write out a lot 
of 32-bit floats and we noticed that at some point recently yaml-cpp started 
forcing 15 digits of precision on numerical outputs.  (See src/emitter.cpp : 
line 661).   While this makes sense for double values, it's not always 
preferable for float types, as most of the right-most digits will be garbage.

Example: 6.01 -> 6.01000022888184

I'm not sure what the best solutions is, possibilities include:
* using a type-aware precision
* Allowing the user to control the precision explicitly (A new flow control 
type?)

But it would probably make sense to allow for some control of the issue. We're 
using yaml files to allow for 'pretty' output, and this runs counter to that 
philosophy.

Thanks!

Original issue reported on code.google.com by jeremy.s...@gmail.com on 17 May 2011 at 11:20

@GoogleCodeExporter
Copy link
Author

I agree, thanks for bringing this up! I've been playing around with some 
syntax, and I'm not sure what the best solution is either. Some issues:

1. Default values. I'd imagine that precision of 6 would be best for floats, 
and 15 for doubles?
2. Global overrides. Could you imagine a situation where you'd want to override 
only the default for floats, but not for doubles? Is it even desirable to 
override the defaults?
3. Local overrides. Something like

emitter << YAML::Precision(2) << 1.234;

produces 1.23.

If we just had better defaults (6 and 15, say), could we just bypass the 
setprecision stuff? If someone really wants fine-grained control 
(printf-style), then they probably would be better off manually doing it, 
rather than using built-in yaml-cpp controls.

What do you think?

Original comment by jbe...@gmail.com on 29 May 2011 at 5:16

  • Changed state: Accepted
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Sounds good. I agree that having better defaults will solve the issue on my 
end.  As for '6' for float default, is that what you get with ostreams by 
default for the float type? We should probably match the default precision we 
used to have (for that type).

Thinking further, if we ever would like to control the precision live in the 
stream it could be done with per-type options.
emitter << YAML::FloatPrecision(2) << 1.234;
emitter << YAML::DoublePrecision(2) << 1.234;

Original comment by jeremy.s...@gmail.com on 31 May 2011 at 4:51

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I don't thing the std::ostream precision distinguishes between floats and 
doubles - and I think it defaults to 6.

What would

emitter << YAML::FloatPrecision(2) << 1.234;

do? (given that 1.234 is a double)

Original comment by jbe...@gmail.com on 1 Jun 2011 at 5:35

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Sure, that would be fine. But the separate float/double default will be the 99% 
case anyways... ;)

Original comment by jeremy.s...@gmail.com on 1 Jun 2011 at 7:55

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Do you have an estimate for how soon this functionality will be available?  
We'd like to rely on the official release of yaml-cpp, but cant until this is 
in there. (we have a .patch file locally forcing the number serialization to 
float precision, but we'd love the more general solution you outline.

Original comment by jeremy.s...@gmail.com on 22 Nov 2011 at 12:44

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I'm not really sure. I'm a grad student, and I'm in the midst of job 
applications, so I haven't really been in YAML mode lately. I've been planning 
to get to a bunch of these things over winter break.

Original comment by jbe...@gmail.com on 22 Nov 2011 at 6:47

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

That's no problem, just was curious. Thanks.

Original comment by jeremy.s...@gmail.com on 22 Nov 2011 at 5:27

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

FYI, this is the project we're using yaml-cpp on:
opencolorio.org

And here's the recent thread discussing this issue...
http://groups.google.com/group/ocio-dev/browse_thread/thread/e17fbf3db74b7074

Original comment by jeremy.s...@gmail.com on 2 Dec 2011 at 9:43

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Cool, thanks for letting me know!

Original comment by jbe...@gmail.com on 2 Dec 2011 at 9:53

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I don't want to nag! Too much :)

But just wanted you to know that this is pretty much the biggest (external) 
hurdle to getting OpenColorIO into distributions that don't allow bundled 
libraries (Fedora, Debian, et al.) 

I appreciate you taking the time to look at this!

Richard (future Fedora OCIO maintainer)

Original comment by hobbes1...@gmail.com on 10 Jan 2012 at 2:36

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Fixed, r20c3badc5ddd.

Sorry for stringing you along :)

Right now the default float precision is 6, and the default double precision is 
15. That probably is enough to solve your problem. There are also manipulators

emitter << YAML::FloatPrecision(3) << 1.2345f;
emitter << YAML::DoublePrecision(3) << 1.2345;

One thing I discovered while doing this: the std::ostream precision manipulator 
counts digits before *and* after the decimal point in normal floating point 
mode. So the above examples would both produce "1.23". I think I'll leave it 
like this, even though it's surprising (to me, at least), because it's 
consistent with the standard library.

Also,

emitter << YAML::FloatPrecision(3) << 1.2345;

won't affect the precision of "1.2345" because it's a double. I think this is 
correct also, but if you disagree, please let me know!

Thanks again for filing this issue!

Original comment by jbe...@gmail.com on 11 Jan 2012 at 8:38

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Awesome, thanks for the fix. We'll check it out. :)

Original comment by jeremy.s...@gmail.com on 11 Jan 2012 at 8:40

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Any estimate on when a 0.2.8 release will be out (with this commit?)

-- Jeremy

Original comment by jeremy.s...@gmail.com on 13 Jan 2012 at 1:43

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

There are three cases I'd like to finish before issuing a new release. (I've 
marked them with a milestone.) Two are features that are reasonably easy, the 
third is just to make sure it builds on Windows (which I can test for sure 
within a week). So I expect by the end of next week I can get a new release out.

By the way, I'm planning to release two new versions: 0.3.0 and 0.5.0. They'll 
be identical, except the 0.3.0 will default to building the old API, and 0.5.0 
will default to building the new API. I'm trying to subtly encourage people to 
start shifting to the new API, but I want to maintain the old one for quite 
some time.

Original comment by jbe...@gmail.com on 13 Jan 2012 at 6:25

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Cool, within a few weeks will work well for us.

A new API? Oh no! Why the update? Is there a docs page you could point us at 
for a preview of what's changing?  I'd sort of hope that a low level library 
like this would have as locked-off as an API as possible.

How soon until we get a yaml-cpp 1.0?

If you'd like to see our use of yaml-cpp (it's relatively modest), and what 
will need to be ported to 0.5, see:

https://github.com/imageworks/OpenColorIO/blob/master/src/core/OCIOYaml.h
https://github.com/imageworks/OpenColorIO/blob/master/src/core/OCIOYaml.cpp
https://github.com/imageworks/OpenColorIO/blob/master/src/core/Config.cpp

(contributors always welcome, if you'd like to use us as a test case for the 
0.5 API). ;)

Original comment by jeremy.s...@gmail.com on 13 Jan 2012 at 8:16

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

oh - just found the docs page for the new API.

Original comment by jeremy.s...@gmail.com on 13 Jan 2012 at 8:17

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

The reason I started a new API is to make it easier to do the most common 
things people are doing (loading from text, loading from files) and make it 
possible to build and edit existing nodes.

(Hopefully you agree it's easier! If you have any comments on the new API, 
please let me know.)

The current API is stabilized (and I'll try to maintain it, with bugfixes, 
etc., for as long as possible), but I like the new API so much better, and it's 
going to be the "default" eventually.

As for 1.0, I sort of view that as a long-term goal (sort of like how some old 
UNIX projects do, like WINE). So to me, 1.0 means *no* more API changes, 100% 
spec compliance (including tag resolution, etc.), and as close to libyaml speed 
as possible (right now we're about half as fast).

As for API stability, the 0.3.x line (old API) won't change the old API (except 
possibly very minor changes), but the 0.5.x line probably will change the new 
API (certainly to expand it). Then 0.6.x and on should have a stable new API.

But the new API is at a place that I'd like to start serious field testing, 
which is why I'm going to make it enabled in a separate release instead of just 
leaving it turned off and relying on people to turn it on themselves.

Original comment by jbe...@gmail.com on 13 Jan 2012 at 8:37

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Cool, thanks for the update.  If the new API is still in flux, I think we'll 
hold off a bit on trying to port.  But if I get any spare time i'll take a look 
and check it out. :)

Original comment by jeremy.s...@gmail.com on 13 Jan 2012 at 10:21

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

FYI, I just released 0.3.0 :)

Original comment by jbe...@gmail.com on 21 Jan 2012 at 9:00

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Just bumped up to 3.0, and have 2 compile warnings from yaml-cpp

include/yaml-cpp/binary.h:21: warning: declaration of 'size' shadows a member 
of 'this'
include/yaml-cpp/binary.h:21: warning: declaration of 'data' shadows a member 
of 'this'

Original comment by jeremy.s...@gmail.com on 25 Jan 2012 at 10:17

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

The other issue I noticed is that in 3.0, dictionarys where the keys do not 
have a value defined now emit empty strings.  Is this anticipated?

yaml-cpp-r482:

  - !<ColorSpace>
    name: lm10
    equalitygroup: 


yaml-cpp-3.0:

  - !<ColorSpace>
    name: lm10
    equalitygroup: ""

Original comment by jeremy.s...@gmail.com on 25 Jan 2012 at 11:09

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Sorry about those warnings, that's me being lazy and not testing small changes 
on other platforms. Fixed, r4229ba563708.

As for the dictionary question, I'm sorry, I'm a little confused. What are you 
emitting? (e.g., std::map, a YAML::Node, something else?)

Original comment by jbe...@gmail.com on 25 Jan 2012 at 11:54

  • Added labels: ****
  • Removed labels: ****

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant