Implement scientific notation for double-to-string #1653

Merged
merged 1 commit into from Dec 18, 2012

Conversation

Projects
None yet
3 participants
Contributor

strk commented Dec 17, 2012

Also fixes tests for 1e5 expecting fixed precision rather than
scientific notation (stringstream gives scientific notation indeed)

The only still failing test now is the one having less than 16
significant digits of precision, due to the boost bug:
https://svn.boost.org/trac/boost/ticket/7785

ref: #1632

Sandro Santilli Implement scientific notation for double-to-string
Also fixes tests for 1e5 expecting fixed precision rather than
scientific notation (stringstream gives scientific notation indeed)

The only still failing test now is the one having less than 16
significant digits of precision, due to the boost bug:
https://svn.boost.org/trac/boost/ticket/7785
e8b7b82
Owner

springmeyer commented Dec 17, 2012

Do you have a sense of the performance impact of this change? How it compares to stringstream? How it compares to current impl with karma that does not worry about scientific notation?

Contributor

strk commented Dec 17, 2012

I didn't profile the code.

Contributor

strk commented Dec 17, 2012

Quick timing test:

With no -O shows stringstream is faster:
4.5 vs. 6.4 seconds to run 1e5 times each of the conversions
also wrote in the test

With -O2 stringstream takes 4.3 and karma 2.2 secs.
The old karma (before I made the first patch) took 2.0.

Owner

springmeyer commented Dec 17, 2012

great, that is a relief. We moved to karma specifically to avoid the overhead of stringstream, locale, etc.

I will let @artemp review and merge.

@artemp artemp added a commit that referenced this pull request Dec 18, 2012

@artemp artemp Merge pull request #1653 from strk/master-float-scientific
Implement scientific notation for double-to-string
e7891a9

@artemp artemp merged commit e7891a9 into mapnik:master Dec 18, 2012

1 check failed

default The Travis build failed
Details
Contributor

strk commented Dec 18, 2012

@artemp: thanks ! Could you also take care of the one for 2.1.x ? It is #1631, up to date with this one.

Owner

artemp commented Dec 18, 2012

@strk - done

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