Ann: Floating Point Utilities Review starts today
The review of Johan Rade's floating point utilities starts today. Code and docs can be downloaded from : http://www.boost-consulting.com/vault/index.php?action=downloadfile&filename=floating_point_utilities_v3.zip&directory=Math%20-%20Numerics& The library consists of three parts: 1) Floating point classification routines: these are optimised implementations of the C99 and C++ TR1 functions fpclassify, isinf, isnan, isnormal and isfinite. From Boost-1.35 onwards these are already a part of Boost.Math (see http://svn.boost.org/svn/boost/trunk/libs/math/doc/sf_and_dist/html/math_too...) so if accepted the two implementations will get merged. The review here should focus on the implementation used, and testing on whatever platforms you have available - in particular are there any circumstances (compiler optimisation settings etc) where this implementation breaks? 2) Sign manipulation functions: implementations of the C99 and C++ TR1 functions copysign and signbit, plus the changesign function. Two of these (signbit and copysign) are currently undocumented members of Boost.Math, and again the two implementations will get merged if this library is accepted. Again the main focus of the review here is the implementation, and testing thereof especially in the presence of compiler optimisations. 3) C++ locale facets: these will read and write non-finite numbers in a portable and round-trippable way: that is not otherwise possible with current C++ std library implementations. These are particularly useful for number-serialisation for example. Since the design is already specified by the C++ standard for these facets, your review here should focus on implementation, testing, documentation, and perhaps where in Boost these should best be placed if accepted. These look to be a useful collection of utilities, so I'll look forward to your reviews, Regards, John Maddock, Floating Point Utilities Review Manager.
John Maddock wrote:
The review of Johan Rade's floating point utilities starts today.
Code and docs can be downloaded from : http://www.boost-consulting.com/vault/index.php?action=downloadfile&filename=floating_point_utilities_v3.zip&directory=Math%20-%20Numerics&
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of John Maddock Sent: 18 February 2008 17:27 To: Boost mailing list; boost-users Subject: [boost] Ann: Floating Point Utilities Review starts today
The review of Johan Rade's floating point utilities starts today.
Sadly, I am away for the duration of this review, and can't spend time on a proper review, but I'd just like to record that I think this collection is *really, really* important to get things floating-point to work smoothly, correctly, and portably. The failure to get these apparently trivial (but actually rather tricky) things to work right has a been a running sore for years and this collection will put things right. So I vote to accept. Paul --- Paul A Bristow Prizet Farmhouse, Kendal, Cumbria UK LA8 8AB +44 1539561830 & SMS, Mobile +44 7714 330204 & SMS pbristow@hetp.u-net.com
Just a gentle reminder that the review of Johan Rade's floating point utilities code is currently under way, and we could use a few more reviews of these useful utilities ! :-) John Maddock.
The review of Johan Rade's floating point utilities starts today.
Code and docs can be downloaded from : http://www.boost-consulting.com/vault/index.php?action=downloadfile&filename=floating_point_utilities_v3.zip&directory=Math%20-%20Numerics&
The library consists of three parts:
1) Floating point classification routines: these are optimised implementations of the C99 and C++ TR1 functions fpclassify, isinf, isnan, isnormal and isfinite. From Boost-1.35 onwards these are already a part of Boost.Math (see http://svn.boost.org/svn/boost/trunk/libs/math/doc/sf_and_dist/html/math_too...) so if accepted the two implementations will get merged.
The review here should focus on the implementation used, and testing on whatever platforms you have available - in particular are there any circumstances (compiler optimisation settings etc) where this implementation breaks?
2) Sign manipulation functions: implementations of the C99 and C++ TR1 functions copysign and signbit, plus the changesign function. Two of these (signbit and copysign) are currently undocumented members of Boost.Math, and again the two implementations will get merged if this library is accepted.
Again the main focus of the review here is the implementation, and testing thereof especially in the presence of compiler optimisations.
3) C++ locale facets: these will read and write non-finite numbers in a portable and round-trippable way: that is not otherwise possible with current C++ std library implementations. These are particularly useful for number-serialisation for example.
Since the design is already specified by the C++ standard for these facets, your review here should focus on implementation, testing, documentation, and perhaps where in Boost these should best be placed if accepted.
These look to be a useful collection of utilities, so I'll look forward to your reviews,
Regards,
John Maddock,
Floating Point Utilities Review Manager.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
What is your evaluation of the design? It is very straightforward, being basically a bag of functions :). Having said that, it would be easier to use the facets in "Facets for Infinity and NaN" if I could call a single function that does the work in each of the three examples. The examples seem to indicate boilerplate code that will be repeated by most, if not all, users. It therefore make sense to me to provide convenience functions that do that work. For instance, this: locale old_locale; locale tmp_locale(old_locale, new nonfinite_num_put<char>); locale new_locale(tmp_locale, new nonfinite_num_get<char>); stringstream ss; ss.imbue(new_locale); would be easier to accomplish with a convenience function like this: stringstream ss; reasonable_function_name(ss); If there is a technical reason why this is not possible, that would be helpful to note this near the examples. What is your evaluation of the implementation? I didn't examine it too closely. What I did look at looked fine. What is your evaluation of the documentation? It is adequate. It would be better if it were done in QuickBook. The one (small) complaint I have is this. Since the rationale for creating the Sign Bit functions (as stated in the Sign Bit intro) includes applicability to 0 and NaN, it would be nice to see those represented in the examples. What is your evaluation of the potential usefulness of the library? Very useful. I especially like the facets bit. I have written Spirit parsers that are cognizant of NaNs and infinity. It would be great not to have to do this in future. Did you try to use the library? With what compiler? Did you have any problems? Yes, with GCC 4.1.0 -- no problems. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent about 3 hours looking at the docs & implementation, and excersizing the library in test code. Are you knowledgeable about the problem domain? Yes. I regularly do numeric work. Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Yes. Zach Laine
Zach Laine wrote:
What is your evaluation of the design?
It is very straightforward, being basically a bag of functions :). Having said that, it would be easier to use the facets in "Facets for Infinity and NaN" if I could call a single function that does the work in each of the three examples. The examples seem to indicate boilerplate code that will be repeated by most, if not all, users. It therefore make sense to me to provide convenience functions that do that work. For instance, this:
locale old_locale; locale tmp_locale(old_locale, new nonfinite_num_put<char>); locale new_locale(tmp_locale, new nonfinite_num_get<char>);
stringstream ss; ss.imbue(new_locale);
would be easier to accomplish with a convenience function like this:
stringstream ss; reasonable_function_name(ss);
If there is a technical reason why this is not possible, that would be helpful to note this near the examples.
That is of course possible, but I'm not convinced it is desirable. 1. I like to keep the interface minimal. 2. A few calls to C++ standard library functions may be easier to read than a single call to a custom function. [snip]
What is your evaluation of the documentation?
It is adequate. It would be better if it were done in QuickBook.
That will be taken care of.
Since the rationale for creating the Sign Bit functions (as stated in the Sign Bit intro) includes applicability to 0 and NaN, it would be nice to see those represented in the examples.
I have never had any use for that particular functionality myself. But it is in TR1. [snip] Thanks for the review! --Johan
AMDG Johan Råde wrote:
Zach Laine wrote:
What is your evaluation of the design?
It is very straightforward, being basically a bag of functions :). Having said that, it would be easier to use the facets in "Facets for Infinity and NaN" if I could call a single function that does the work in each of the three examples. The examples seem to indicate boilerplate code that will be repeated by most, if not all, users. It therefore make sense to me to provide convenience functions that do that work. For instance, this:
locale old_locale; locale tmp_locale(old_locale, new nonfinite_num_put<char>); locale new_locale(tmp_locale, new nonfinite_num_get<char>);
stringstream ss; ss.imbue(new_locale);
would be easier to accomplish with a convenience function like this:
stringstream ss; reasonable_function_name(ss);
If there is a technical reason why this is not possible, that would be helpful to note this near the examples.
That is of course possible, but I'm not convinced it is desirable. 1. I like to keep the interface minimal. 2. A few calls to C++ standard library functions may be easier to read than a single call to a custom function.
In any case I think that such a function does not belong in this library. It would be better to have a separate generic component. template<class Stream, class Facet...> void add_facet_to_stream_locale(Stream&, const Facet&... facet); In Christ, Steven Watanabe
Le lundi 18 février 2008 à 17:27 +0000, John Maddock a écrit :
The review of Johan Rade's floating point utilities starts today.
The library consists of three parts:
1) Floating point classification routines: these are optimised implementations of the C99 and C++ TR1 functions fpclassify, isinf, isnan, isnormal and isfinite.
Concerning isinf, I'm missing the feature of the glibc C library. Since C99/Posix mandates an int as the return type (of the non-macro version), the glibc returns 1 or -1 depending on the sign of the infinity. This is quite nice, and compatible with the standard. Due to the way the code is written (memcpy), it is compliant with the following part of the C99 standard: "First, an argument represented in a format wider than its semantic type is converted to its semantic type. Then determination is based on the type of the argument." But it may seem like a stroke of luck. So perhaps a big "careful!" comment should be written inside the code, so that this property is not inadvertently lost. In the portability section of the documentation, please do not talk about "IEEE 754 extended double precision with 15 exponent bits". It is quite misleading. The standard actually defines a binary interchange format that has a 15-bit exponent. Its official name is "binary128", its common name is "quad(ruple)" precision. There are no longer things like "extended double" formats (since "extended single" got scrapped). So if you really mean binary128, please write it clearly. The fp_traits.hpp file use BOOST_STATIC_CONSTANT (hence potentially an enumeration) with constants that are not representable as int (0x800..). I don't remember the precise details, but I know there is a disaster bound to happen. I think a simple workaround is to just use 0x80000000u instead (notice the "u"), but I can't vouch it is sufficient. As a matter of fact, I would also mark the other big constants as unsigned at line 345 and 349, just to be on the correct/safe side.
2) Sign manipulation functions: implementations of the C99 and C++ TR1 functions copysign and signbit, plus the changesign function. Two of these (signbit and copysign) are currently undocumented members of Boost.Math, and again the two implementations will get merged if this library is accepted.
I am missing the point of the changesign function. What is its rationale? The documentation talks about copysign(x, signbit(x) ? 1 : -1) but not about -x. So I am a bit lost.
3) C++ locale facets: these will read and write non-finite numbers in a portable and round-trippable way: that is not otherwise possible with current C++ std library implementations. These are particularly useful for number-serialisation for example.
It may be worth writing somewhere in the documentation that the library is also compliant with the IEEE 754R standard on floating-point arithmetic. Indeed, as far as I can tell, the only missing features are "should" features: - a signaling NaN should produce, either "snan", or "nan" with invalid signaled - "snan" should be supported in input and it should produce, either a signaling NaN, or a quiet NaN with invalid signaled I have never been a strong believer in the greatness of signaling NaNs anyway, so I don't really miss them. Another missing feature is the exact round-trip, but one may argue that this library is not the one to provide this mandatory feature. Therefore, since it doesn't provide exact conversion, it doesn't have to support output of NaN payloads. (Note that the round-trip, however, is allowed to lose the signaling part of the encoding, as already written above.) What is the rationale for not enabling signed_zero by default? Concerning the code, the put_impl function contains a lot of code duplication. You should factor the code about the sign inside the put_num_and_fill function. It would make the code simpler. Best regards, Guillaume
Guillaume Melquiond wrote:
Concerning isinf, I'm missing the feature of the glibc C library. Since C99/Posix mandates an int as the return type (of the non-macro version), the glibc returns 1 or -1 depending on the sign of the infinity. This is quite nice, and compatible with the standard.
I'll have to think about that. Spontaneously I'm a bit sceptical of functions that do several different things at the same time.
Due to the way the code is written (memcpy), it is compliant with the following part of the C99 standard: "First, an argument represented in a format wider than its semantic type is converted to its semantic type. Then determination is based on the type of the argument." But it may seem like a stroke of luck. So perhaps a big "careful!" comment should be written inside the code, so that this property is not inadvertently lost.
I don't understand. I'm not a C++ guru. Could you please elaborate.
In the portability section of the documentation, please do not talk about "IEEE 754 extended double precision with 15 exponent bits". It is quite misleading. The standard actually defines a binary interchange format that has a 15-bit exponent. Its official name is "binary128", its common name is "quad(ruple)" precision. There are no longer things like "extended double" formats (since "extended single" got scrapped). So if you really mean binary128, please write it clearly.
I'll change that.
The fp_traits.hpp file use BOOST_STATIC_CONSTANT (hence potentially an enumeration) with constants that are not representable as int (0x800..). I don't remember the precise details, but I know there is a disaster bound to happen. I think a simple workaround is to just use 0x80000000u instead (notice the "u"), but I can't vouch it is sufficient. As a matter of fact, I would also mark the other big constants as unsigned at line 345 and 349, just to be on the correct/safe side.
I'll change that.
I am missing the point of the changesign function. What is its rationale? The documentation talks about copysign(x, signbit(x) ? 1 : -1) but not about -x. So I am a bit lost.
Is -x guaranteed to change the sign of zero and NaN? If it is, then I agree that the function is superfluous.
It may be worth writing somewhere in the documentation that the library is also compliant with the IEEE 754R standard on floating-point arithmetic. Indeed, as far as I can tell, the only missing features are "should" features:
- a signaling NaN should produce, either "snan", or "nan" with invalid signaled - "snan" should be supported in input and it should produce, either a signaling NaN, or a quiet NaN with invalid signaled
I have never been a strong believer in the greatness of signaling NaNs anyway, so I don't really miss them.
I'm not a big fan of signaling NaN either. There is the additional complication that the binary representation of quiet/signaling NaN is not standardized. For instance, a bit pattern that represents a quiet NaN on an Intel or PowerPC processor, would represent a signaling NaN on a MIPS processor. Therefore it would be tricky to provide a portable implementation.
Another missing feature is the exact round-trip, but one may argue that this library is not the one to provide this mandatory feature. Therefore, since it doesn't provide exact conversion, it doesn't have to support output of NaN payloads. (Note that the round-trip, however, is allowed to lose the signaling part of the encoding, as already written above.)
I have no plans to implement that.
What is the rationale for not enabling signed_zero by default?
I think most users of the library will not want it enabled by default. They may not care much about the fine details of floating point arithmetic. Insted they may just want to prevent crashes when deserializing text archives that contain (by design or by accident) infinity or NaN.
Concerning the code, the put_impl function contains a lot of code duplication. You should factor the code about the sign inside the put_num_and_fill function. It would make the code simpler.
I will look into that. Thank you for your detailed comments! Johan Råde
Le lundi 25 février 2008 à 18:25 +0100, Johan Råde a écrit :
Due to the way the code is written (memcpy), it is compliant with the following part of the C99 standard: "First, an argument represented in a format wider than its semantic type is converted to its semantic type. Then determination is based on the type of the argument." But it may seem like a stroke of luck. So perhaps a big "careful!" comment should be written inside the code, so that this property is not inadvertently lost.
I don't understand. I'm not a C++ guru. Could you please elaborate.
It does not have anything to do with C++, but with the way some system handle floating-point numbers. For instance, x87-style arithmetic uses an internal exponent field of 15 bits. Let us suppose your code is: double x = 1e300; fpclassify(x * x); For the processor, 1e600 is a normal number since it does not overflow its internal representation, yet it has (supposedly) the type "double", so it should be an infinity. That's why the standard asks for a conversion to the "semantic" type beforehand. Another example would be a system where all the computations are done in double precision, yet single precision is available as a storage format. So the compiler would store the intermediate values as double precision number, even if they are single precision from a type point of view. In case it is still not clear, here is an example of a code that does not portably follow the C99 standard, but that works on any system where the computation format is the same as the semantic type: if (x != x) return FP_NAN; else if (x - x == 0) ... else return FP_INFINITE; Once again, the reason is that x may be stored internally in a wider format, so x - x will be 0 although it should be NaN in case of overflow.
I am missing the point of the changesign function. What is its rationale? The documentation talks about copysign(x, signbit(x) ? 1 : -1) but not about -x. So I am a bit lost.
Is -x guaranteed to change the sign of zero and NaN? If it is, then I agree that the function is superfluous.
For NaN, the question is a bit tricky, since the 754 standard explicitly states that the sign bit is not part of the payload of NaN, so it may just be a bit that the processor randomly flips over time. Fortunately, processor designers do not add this kind of feature to their product (or at least they do not anymore). Note that it isn't related to negate; The sign of NaN is a slippery topic in general. Other than that, the negate operation is clearly defined by the 754 standard (just flip the sign bit). Now, the C/C++ standard does not prevent compiler designers to be more imaginative. So they could slow down the generated code just so that it does not follow the floating-point standard. But I don't think the users of such a random compiler really care about the sign of zero... Back to the topic at hand, I don't know of any architecture/compiler where negating a number does not flip the sign bit of the number, so -x works both for zero and NaN.
What is the rationale for not enabling signed_zero by default?
I think most users of the library will not want it enabled by default. They may not care much about the fine details of floating point arithmetic. Insted they may just want to prevent crashes when deserializing text archives that contain (by design or by accident) infinity or NaN.
I should have added the following: On all the systems I have access to (various *nix flavors), the default behavior is to display the sign of negative zero. So I don't know if that is what the users expect. But I'm quite sure that users don't expect a change in the way zero is displayed, when they use a facet dedicated to "nonfinite" things. So perhaps there should be three states: signed_zero, unsigned_zero, default_zero. And default_zero being the default state, which just delegate the output to the underlying system. Best regards, Guillaume
Guillaume Melquiond wrote:
Le lundi 25 février 2008 à 18:25 +0100, Johan Råde a écrit :
What is the rationale for not enabling signed_zero by default? I think most users of the library will not want it enabled by default. They may not care much about the fine details of floating point arithmetic. Insted they may just want to prevent crashes when deserializing text archives that contain (by design or by accident) infinity or NaN.
I should have added the following: On all the systems I have access to (various *nix flavors), the default behavior is to display the sign of negative zero. So I don't know if that is what the users expect. But I'm quite sure that users don't expect a change in the way zero is displayed, when they use a facet dedicated to "nonfinite" things.
On VC71, the default behavior is not to display the sign of negative zero. I'll have to think about this. --Johan
I am missing the point of the changesign function. What is its rationale? The documentation talks about copysign(x, signbit(x) ? 1 : -1) but not about -x. So I am a bit lost.
Is -x guaranteed to change the sign of zero and NaN? If it is, then I agree that the function is superfluous.
Back to the topic at hand, I don't know of any architecture/compiler where negating a number does not flip the sign bit of the number, so -x works both for zero and NaN.
I just found out that for the ia32 architecture, unary minus flips the signbit of NaN if you use the x87 units, but not if you use the SSE units. So the changesign function is useful. I should point this out in the docs. --Johan
Le lundi 25 février 2008 à 22:08 +0100, Johan Råde a écrit :
I am missing the point of the changesign function. What is its rationale? The documentation talks about copysign(x, signbit(x) ? 1 : -1) but not about -x. So I am a bit lost.
Is -x guaranteed to change the sign of zero and NaN? If it is, then I agree that the function is superfluous.
Back to the topic at hand, I don't know of any architecture/compiler where negating a number does not flip the sign bit of the number, so -x works both for zero and NaN.
I just found out that for the ia32 architecture, unary minus flips the signbit of NaN if you use the x87 units, but not if you use the SSE units. So the changesign function is useful.
Sorry to disappoint you, but you probably have not found anything but a compiler optimization. As I explained in a previous mail, the sign bit is not part of the NaN payload. So a standard-compliant compiler is allowed to replace -NaN(17) by NaN(17) in the code. If you put a "volatile" between the NaN and the negation, you will probably get your sign back. It doesn't have anything to do with the arithmetic units. As a matter of fact, the SSE units do not provide any "negate" opcode, so compilers generate a "xor" opcode instead. You will have a hard time convincing anybody that the xor instruction is failing to flip the sign bit. So I suggest you take a look at the assembly output of your compiler and check what the generated code actually look like. Best regards, Guillaume
Guillaume Melquiond wrote:
Le lundi 25 février 2008 à 22:08 +0100, Johan Råde a écrit :
I am missing the point of the changesign function. What is its rationale? The documentation talks about copysign(x, signbit(x) ? 1 : -1) but not about -x. So I am a bit lost. Is -x guaranteed to change the sign of zero and NaN? If it is, then I agree that the function is superfluous. Back to the topic at hand, I don't know of any architecture/compiler where negating a number does not flip the sign bit of the number, so -x works both for zero and NaN. I just found out that for the ia32 architecture, unary minus flips the signbit of NaN if you use the x87 units, but not if you use the SSE units. So the changesign function is useful.
Sorry to disappoint you, but you probably have not found anything but a compiler optimization.
As I explained in a previous mail, the sign bit is not part of the NaN payload. So a standard-compliant compiler is allowed to replace -NaN(17) by NaN(17) in the code. If you put a "volatile" between the NaN and the negation, you will probably get your sign back. It doesn't have anything to do with the arithmetic units.
As a matter of fact, the SSE units do not provide any "negate" opcode, so compilers generate a "xor" opcode instead. You will have a hard time convincing anybody that the xor instruction is failing to flip the sign bit.
So I suggest you take a look at the assembly output of your compiler and check what the generated code actually look like.
If I enable SSE2 on VC71 and optimize the code for speed (/O2), then -std::numeric_limits<double>::quiet_nan() returns a positive NaN (binary expansion 7ff8000000000000), while boost::math::changesign(std::numeric_limits<double>::quiet_nan()) returns a negative NaN (binary expansion fff8000000000000). If I turn off the compiler optimization, then both return a negative NaN. Conclusion: boost::math::changesign is useful. --Johan
Johan Råde wrote:
If I enable SSE2 on VC71 and optimize the code for speed (/O2), then
-std::numeric_limits<double>::quiet_nan()
returns a positive NaN (binary expansion 7ff8000000000000), while
boost::math::changesign(std::numeric_limits<double>::quiet_nan())
returns a negative NaN (binary expansion fff8000000000000).
If I turn off the compiler optimization, then both return a negative NaN.
Conclusion: boost::math::changesign is useful.
Well, if that's the only use, then personally I won't be using it, what use is a negative NaN over a positive one? This feels like a very borderline use case to me, but maybe I'm missing something? John.
John Maddock wrote:
Johan Råde wrote:
If I enable SSE2 on VC71 and optimize the code for speed (/O2), then
-std::numeric_limits<double>::quiet_nan()
returns a positive NaN (binary expansion 7ff8000000000000), while
boost::math::changesign(std::numeric_limits<double>::quiet_nan())
returns a negative NaN (binary expansion fff8000000000000).
If I turn off the compiler optimization, then both return a negative NaN.
Conclusion: boost::math::changesign is useful.
Well, if that's the only use, then personally I won't be using it, what use is a negative NaN over a positive one? This feels like a very borderline use case to me, but maybe I'm missing something?
Paul Bristow wanted support for negative NaN. Unfortunately he is gone this week, and can not comment on this issue. I think he had in mind that some users might want to record extra information about the NaN through the signbit. But as Guillaume has explained, that may not be the best way to add information to the NaN. Personally I have no strong opinion about this. Currently negative NaN is formatted as "-nan". Having changesign made it easier to implement and write tests for this functionality. To me it seems logical to either keep both the formatting of negative NaN as "-nan" and the changesign function, or to drop both. Either way is fine with me. --Johan
John Maddock wrote:
Johan Råde wrote:
[snip]
Conclusion: boost::math::changesign is useful.
Well, if that's the only use, then personally I won't be using it, what use is a negative NaN over a positive one? This feels like a very borderline use case to me, but maybe I'm missing something?
After thinking more about this, and especially taking Guillaume's comments into account, I've decided to drop both signed NaN formatting and the changesign function. If anyone needs to convey extra information with a NaN, then the nan(...) format should be used. I will not implement that, but if anyone needs it, he is free to extend this library. --Johan
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Guillaume Melquiond Sent: 25 February 2008 21:49 To: boost@lists.boost.org Subject: Re: [boost] Floating Point Utilities Review starts today
As I explained in a previous mail, the sign bit is not part of the NaN payload. So a standard-compliant compiler is allowed to replace -NaN(17) by NaN(17) in the code.
OK - you obviously know vastly more than I about this - if the Standard says this, (unhelpful or not) then it would be a bad idea to try to use the sign bit to carry any information. But this still me leaves wondering about the original (very common) use case - you have a 'database' (and you want to be able to serialize - and back (of course, at least for systems using the same FP layout for floats and doubles). Without loopback it's difficult to validate its contents. It would be extremely useful to be able to distiguish between values that never existed - 'missing values' and those that some computation results in a NaN. Can you suggest how to achieve a 'missing value'? Surely we can do better than 'Enter 999999.9 to end input' - using a single FP bit pattern for 'missing value'? Do we have to accept storing a bool with each value - and take a serious hit in 'database' size and the cost of its transfer? Do we use other bit(s) in the NaN payload? Paul --- Paul A Bristow Prizet Farmhouse, Kendal, Cumbria UK LA8 8AB +44 1539561830 & SMS, Mobile +44 7714 330204 & SMS pbristow@hetp.u-net.com
Johan Råde wrote:
Guillaume Melquiond wrote:
Concerning isinf, I'm missing the feature of the glibc C library. Since C99/Posix mandates an int as the return type (of the non-macro version), the glibc returns 1 or -1 depending on the sign of the infinity. This is quite nice, and compatible with the standard.
I'll have to think about that. Spontaneously I'm a bit sceptical of functions that do several different things at the same time.
Indeed, and the TR1 version of isinf returns a bool. John.
3) C++ locale facets: these will read and write non-finite numbers in a portable and round-trippable way: that is not otherwise possible with current C++ std library implementations. These are particularly useful for number-serialisation for example.
There are quite a few test failures from these, I'm hoping these are related to these facets rather than the underlying utility code: MSVC-8 (debug) Running 8 test cases... nonfinite_num_facets/legacy_test.cpp(120): error in "legacy_test": check (isnan) (b11) failed nonfinite_num_facets/legacy_test.cpp(138): error in "legacy_test": check ss.rdst ate() == std::ios_base::eofbit failed nonfinite_num_facets/legacy_test.cpp(120): error in "legacy_test": check (isnan) (b11) failed nonfinite_num_facets/legacy_test.cpp(138): error in "legacy_test": check ss.rdst ate() == std::ios_base::eofbit failed nonfinite_num_facets/legacy_test.cpp(120): error in "legacy_test": check (isnan) (b11) failed nonfinite_num_facets/legacy_test.cpp(138): error in "legacy_test": check ss.rdst ate() == std::ios_base::eofbit failed nonfinite_num_facets/legacy_test.cpp(120): error in "legacy_test": check (isnan) (b11) failed nonfinite_num_facets/legacy_test.cpp(138): error in "legacy_test": check ss.rdst ate() == std::ios_base::eofbit failed nonfinite_num_facets/legacy_test.cpp(120): error in "legacy_test": check (isnan) (b11) failed nonfinite_num_facets/legacy_test.cpp(138): error in "legacy_test": check ss.rdst ate() == std::ios_base::eofbit failed nonfinite_num_facets/legacy_test.cpp(120): error in "legacy_test": check (isnan) (b11) failed nonfinite_num_facets/legacy_test.cpp(138): error in "legacy_test": check ss.rdst ate() == std::ios_base::eofbit failed *** 12 failures detected in test suite "Master Test Suite" MSVC-8 (release + SSE2) Running 8 test cases... nonfinite_num_facets/basic_test.cpp(167): error in "basic_test": check ss.str() == s failed nonfinite_num_facets/basic_test.cpp(182): error in "basic_test": check (signbit) (b2) failed nonfinite_num_facets/basic_test.cpp(184): error in "basic_test": check (signbit) (b4) failed nonfinite_num_facets/basic_test.cpp(167): error in "basic_test": check ss.str() == s failed nonfinite_num_facets/basic_test.cpp(182): error in "basic_test": check (signbit) (b2) failed nonfinite_num_facets/basic_test.cpp(184): error in "basic_test": check (signbit) (b4) failed nonfinite_num_facets/basic_test.cpp(167): error in "basic_test": check ss.str() == s failed nonfinite_num_facets/basic_test.cpp(182): error in "basic_test": check (signbit) (b2) failed nonfinite_num_facets/basic_test.cpp(184): error in "basic_test": check (signbit) (b4) failed nonfinite_num_facets/basic_test.cpp(167): error in "basic_test": check ss.str() == s failed nonfinite_num_facets/basic_test.cpp(182): error in "basic_test": check (signbit) (b2) failed nonfinite_num_facets/basic_test.cpp(184): error in "basic_test": check (signbit) (b4) failed nonfinite_num_facets/basic_test.cpp(167): error in "basic_test": check ss.str() == s failed nonfinite_num_facets/basic_test.cpp(182): error in "basic_test": check (signbit) (b2) failed nonfinite_num_facets/basic_test.cpp(184): error in "basic_test": check (signbit) (b4) failed nonfinite_num_facets/basic_test.cpp(167): error in "basic_test": check ss.str() == s failed nonfinite_num_facets/basic_test.cpp(182): error in "basic_test": check (signbit) (b2) failed nonfinite_num_facets/basic_test.cpp(184): error in "basic_test": check (signbit) (b4) failed nonfinite_num_facets/lexical_cast_test.cpp(86): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a9) == s9 failed nonfinite_num_facets/lexical_cast_test.cpp(87): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a10) == s10 failed nonfinite_num_facets/lexical_cast_test.cpp(86): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a9) == s9 failed nonfinite_num_facets/lexical_cast_test.cpp(87): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a10) == s10 failed nonfinite_num_facets/lexical_cast_test.cpp(86): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a9) == s9 failed nonfinite_num_facets/lexical_cast_test.cpp(87): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a10) == s10 failed nonfinite_num_facets/lexical_cast_test.cpp(86): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a9) == s9 failed nonfinite_num_facets/lexical_cast_test.cpp(87): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a10) == s10 failed nonfinite_num_facets/lexical_cast_test.cpp(86): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a9) == s9 failed nonfinite_num_facets/lexical_cast_test.cpp(87): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a10) == s10 failed nonfinite_num_facets/lexical_cast_test.cpp(86): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a9) == s9 failed nonfinite_num_facets/lexical_cast_test.cpp(87): error in "lexical_cast_test": ch eck lexical_cast<std::basic_string<CharType> >(a10) == s10 failed *** 30 failures detected in test suite "Master Test Suite" Perhaps perversely, VC8 in release mode, but without the /arch:SSE2 option passes all the tests. Intel-10.1 has similar failures (except in SSE2 mode!), so this may be iostream's related. Full review to follow.... John.
John Maddock wrote:
1) Floating point classification routines: these are optimised implementations of the C99 and C++ TR1 functions fpclassify, isinf, isnan, isnormal and isfinite. From Boost-1.35 onwards these are already a part of Boost.Math (see http://svn.boost.org/svn/boost/trunk/libs/math/doc/sf_and_dist/html/math_too...) so if accepted the two implementations will get merged.
The review here should focus on the implementation used, and testing on whatever platforms you have available - in particular are there any circumstances (compiler optimisation settings etc) where this implementation breaks?
These look fine to me, and as Johan mentioned, any breakages should be fixed in endian.hpp. Sorry to bring this up again (!), but I've been trying to get my head around the aliasing rules and what is and isn't allowed, in particular by following the rather useful article here: http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_alias...
From this it appears that your current code is perfectly fine, however I still can't help feeling that we could do better than call an external function (memcpy) to move 4 or 8 bytes. The suggestion in the article is to use a union to move from one type to another:
union float_to_int32 { float f; boost::uint32_t i; }; boost::uint32_t get_bits(float f) { float_to_int32 u; u.f = f; return u.i; } I can't claim to have tested exhaustively, but modifying get_bits and set_bits to use this method seems to work OK for VC8&9 and Minwg GCC. Thoughts? John.
Le mardi 26 février 2008 à 14:04 +0000, John Maddock a écrit :
Sorry to bring this up again (!), but I've been trying to get my head around the aliasing rules and what is and isn't allowed, in particular by following the rather useful article here: http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_alias...
From this it appears that your current code is perfectly fine, however I still can't help feeling that we could do better than call an external function (memcpy) to move 4 or 8 bytes. The suggestion in the article is to use a union to move from one type to another:
(I have not read the article, so perhaps the following was already discussed there.) Using an union this way also breaks strict aliasing. Fortunately, at least for GCC, this breakage is explicitly documented as being an extension supported by the compiler. So it is guaranteed to work just fine. But I don't know if it is also a documented feature for other compilers, or if it may break once they strengthen their optimization algorithms. You were concerned with calling memcpy, but at least for GCC, the compiler recognizes this function. As a consequence, it does not perform any memory copy at all and directly moves data from integer registers to float registers, if permitted by the processor. So the generated code is optimal (or at least not worse than what the compiler usually generates). Again, I have no idea how other compilers behave. Summary: With GCC, both the union and the memcpy generates the exact same assembly code. Both are documented as being supported. But the second one is the only one that is compliant with the C++ standard. Best regards, Guillaume
John Maddock wrote:
Sorry to bring this up again (!), but I've been trying to get my head around the aliasing rules and what is and isn't allowed, in particular by following the rather useful article here: http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_alias...
From this it appears that your current code is perfectly fine, however I still can't help feeling that we could do better than call an external function (memcpy) to move 4 or 8 bytes.
When you compile on Visual Studio, with the compiler option /Oi (or /O2), then the compiler treats memcpy as a compiler intrinsic (i.e. essentially as a key word) and not as an external function; see http://msdn2.microsoft.com/en-us/library/tzkfha43.aspx I looked at assembler output a year ago, and, if I remember right, a memcpy of four bytes was compiled to a single move instruction. The suggestion in the article is to
use a union to move from one type to another:
union float_to_int32 { float f; boost::uint32_t i; };
boost::uint32_t get_bits(float f) { float_to_int32 u; u.f = f; return u.i; }
I can't claim to have tested exhaustively, but modifying get_bits and set_bits to use this method seems to work OK for VC8&9 and Minwg GCC.
Interesting, but I wouldn't do this without the approval of a pedantic langauge lawyer. I have been bitten by this sort of things in the past. And I'm not sure it would make things faster on Visual Studio anyway. --Johan
----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Johan Råde Sent: 26 February 2008 15:08 To: boost@lists.boost.org Subject: Re: [boost] Ann: Floating Point Utilities Review starts today
John Maddock wrote:
Sorry to bring this up again (!), but I've been trying to get my head around the aliasing rules and what is and isn't allowed, in particular by following the rather useful article here: http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_alias...
From this it appears that your current code is perfectly fine, however I still can't help feeling that we could do better than call an external function (memcpy) to move 4 or 8 bytes.
When you compile on Visual Studio, with the compiler option /Oi (or /O2), then the compiler treats memcpy as a compiler intrinsic (i.e. essentially as a key word) and not as an external function; see http://msdn2.microsoft.com/en-us/library/tzkfha43.aspx I looked at assembler output a year ago, and, if I remember right, a memcpy of four bytes was compiled to a single move instruction.
The suggestion in the article is to
use a union to move from one type to another:
union float_to_int32 { float f; boost::uint32_t i; };
boost::uint32_t get_bits(float f) { float_to_int32 u; u.f = f; return u.i; }
I can't claim to have tested exhaustively, but modifying get_bits and set_bits to use this method seems to work OK for VC8&9 and Minwg GCC.
Interesting, but I wouldn't do this without the approval of a pedantic langauge lawyer. I have been bitten by this sort of things in the past. And I'm not sure it would make things faster on Visual Studio anyway.
I'm not sure I'm a pedantic language lawyer, but the standard says "In a union, at most one of the data members can be active at any time". It is quite clear that experts in CLC++M regard John Maddock's version of setbits and getbits as undefined behaviour. In general, I would regard using undefined behaviour as unacceptable for a boost library. The floating point utilities are different, they pretty much /have/ to use undefined behaviour. They also seem to be taking the correct approach of using #ifdef, and testing on as many platforms as possible (I haven't reviewed them yet). On the other hand, even if floating point utilities have to go through a minefield, there doesn't seem any point in wandering around more than necessary. Particularly if the tested compilers don't actually generate better code for the undefined behaviour version! -- Martin Bonner Senior Software Engineer/Team Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer
In general, I would regard using undefined behaviour as unacceptable for a boost library. The floating point utilities are different, they pretty much /have/ to use undefined behaviour.
I wouldn't say that the library relies on undefined behavior. It relies on the C++ Standard and the IEEE 754 standard, and is guaranteed to work correctly on any platforms that conform to these two standards. (In addition, it uses preprocessor logic to handle platforms with long double implementations that do not conform to the IEEE 754 standard.) --Johan
Martin Bonner wrote:
Interesting, but I wouldn't do this without the approval of a pedantic langauge lawyer. I have been bitten by this sort of things in the past. And I'm not sure it would make things faster on Visual Studio anyway.
I'm not sure I'm a pedantic language lawyer, but the standard says "In a union, at most one of the data members can be active at any time". It is quite clear that experts in CLC++M regard John Maddock's version of setbits and getbits as undefined behaviour. In general, I would regard using undefined behaviour as unacceptable for a boost library.
Does anyone know a modern C++ compiler where aliasing through a union doesn't work? Best regards, Stephan
Martin Bonner wrote:
I'm not sure I'm a pedantic language lawyer, but the standard says "In a union, at most one of the data members can be active at any time". It is quite clear that experts in CLC++M regard John Maddock's version of setbits and getbits as undefined behaviour. In general, I would regard using undefined behaviour as unacceptable for a boost library. The floating point utilities are different, they pretty much /have/ to use undefined behaviour. They also seem to be taking the correct approach of using #ifdef, and testing on as many platforms as possible (I haven't reviewed them yet).
On the other hand, even if floating point utilities have to go through a minefield, there doesn't seem any point in wandering around more than necessary. Particularly if the tested compilers don't actually generate better code for the undefined behaviour version!
Indeed, suggestion withdrawn! :-) It all goes to show why I don't normally mess about at such a low level :-( John.
On Tue, Feb 26, 2008 at 3:04 PM, John Maddock <john@johnmaddock.co.uk> wrote:
John Maddock wrote:
1) Floating point classification routines: these are optimised implementations of the C99 and C++ TR1 functions fpclassify, isinf, isnan, isnormal and isfinite. From Boost-1.35 onwards these are already a part of Boost.Math (see http://svn.boost.org/svn/boost/trunk/libs/math/doc/sf_and_dist/html/math_too...) so if accepted the two implementations will get merged.
The review here should focus on the implementation used, and testing on whatever platforms you have available - in particular are there any circumstances (compiler optimisation settings etc) where this implementation breaks?
These look fine to me, and as Johan mentioned, any breakages should be fixed in endian.hpp.
Sorry to bring this up again (!), but I've been trying to get my head around the aliasing rules and what is and isn't allowed, in particular by following the rather useful article here: http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_alias...
also see: http://mail-index.netbsd.org/tech-kern/2003/08/11/0001.html
From this it appears that your current code is perfectly fine, however I still can't help feeling that we could do better than call an external function (memcpy) to move 4 or 8 bytes. The suggestion in the article is to use a union to move from one type to another:
union float_to_int32 { float f; boost::uint32_t i; };
boost::uint32_t get_bits(float f) { float_to_int32 u; u.f = f; return u.i; }
I can't claim to have tested exhaustively, but modifying get_bits and set_bits to use this method seems to work OK for VC8&9 and Minwg GCC.
GCC 4.2 under x86_64 produces better code with std::memcpy (which is treated as an intrinsic) than with the union trick (compiled with -O3): uint32_t get_bits(float f) { float_to_int32 u; u.f = f; return u.i; } Generates: _Z8get_bitsf: movss %xmm0, -4(%rsp) movl -4(%rsp), %edx movl %edx, %eax ret Which has an useless "movl %edx, %eax". I think that using the union confuses the optimizer. This instead: uint32_t get_bits2(float f) { uint32_t ret; std::memcpy(&ret, &f, sizeof(f)); return ret; } Generates: _Z9get_bits3f: movss %xmm0, -4(%rsp) movl -4(%rsp), %eax ret Which should be optimal (IIRC you can't move from an xmms register to an integer register without passing through memory). Note that the (illegal) code: uint32_t get_bits3(float f) { uint32_t ret = *reinterpret_cast<uint32_t*>(&f); return ret; } Generates the same code as get_bits2 if compiled with -fno-strict-aliasing. Without that flag it miscompiles (rightly) the code. I've tested the code under plain x86 and there is no difference between all 3 functions. So the standard compliant code is also optimal, at least with recent GCCs. -- gpd
On Tue, Feb 26, 2008 at 4:59 PM, Sebastian Redl <sebastian.redl@getdesigned.at> wrote:
Giovanni Piero Deretta wrote:
_Z9get_bits3f: movss %xmm0, -4(%rsp) movl -4(%rsp), %eax ret
Which should be optimal (IIRC you can't move from an xmms register to an integer register without passing through memory).
SSE2: movd %xmm0, %eax
Right, it was the old x87 register stack that doesn't support fp-register/general-purpose-register moves. Anyways, after reading some documentation, it seems that the generated assembly is probably still optimal for K8 and pentium4 which have high latency and very high latency (respectively) xmm register/register moves. My gcc version isn't specifically capable of optimizing for core2, which shouldn't have this limitation. -- gpd
Giovanni Piero Deretta wrote:
also see: http://mail-index.netbsd.org/tech-kern/2003/08/11/0001.html
Thanks, will take a look.
GCC 4.2 under x86_64 produces better code with std::memcpy (which is treated as an intrinsic) than with the union trick (compiled with -O3):
uint32_t get_bits(float f) { float_to_int32 u; u.f = f; return u.i; }
Generates:
_Z8get_bitsf: movss %xmm0, -4(%rsp) movl -4(%rsp), %edx movl %edx, %eax ret
Which has an useless "movl %edx, %eax". I think that using the union confuses the optimizer. This instead:
uint32_t get_bits2(float f) { uint32_t ret; std::memcpy(&ret, &f, sizeof(f)); return ret; }
Generates:
_Z9get_bits3f: movss %xmm0, -4(%rsp) movl -4(%rsp), %eax ret
Which should be optimal (IIRC you can't move from an xmms register to an integer register without passing through memory).
Note that the (illegal) code:
uint32_t get_bits3(float f) { uint32_t ret = *reinterpret_cast<uint32_t*>(&f); return ret; }
Generates the same code as get_bits2 if compiled with -fno-strict-aliasing. Without that flag it miscompiles (rightly) the code. I've tested the code under plain x86 and there is no difference between all 3 functions.
So the standard compliant code is also optimal, at least with recent GCCs.
Very interesting! In that case I think we should document what Johan has now, to avoid this comming up again in the future :-) Thanks! John.
John Maddock wrote:
The review of Johan Rade's floating point utilities starts today.
I haven't had time to do a detailed review, but from the about an hour I have spent with the library, I'm in favor off it. The documentation is adaquite, since there is very little rationale to explain and the whole goal is to implement the TR1 functions (with the slight addition of the consistent serialization). I would like to see it add a couple more examples with some more explanitory text, since I'm not sure everyone looking at the docs will understand how much they need what the library does, but for anyone who already knows that there are problems that need to be solved, it is good. I am also concerned about merging it into the MathTools library. That is certainly where it belongs in a logical sense, but the MathTools library is already huge and the documentation is extensive. This small but sometimes essential library could easily get lost in the flood of information that is the MathTools documentation. Assuming this library is accepted, some thought about how to make this component obvious in the larger documentation is required. I didn't get a chance to try anything beyond the simple examples, but those seem to work on a PPC Mac running OSX 10.5. I plan to use these pieces in other places, but I haven't had the chance, yet. I don't claim to be an expert on the detailed cross platform issues that this library addresses. I'm well versed in computational numerics, but not as well versed on these portability details. I vote in favor of accepting the library. John Phillips
John Phillips wrote:
I am also concerned about merging it into the MathTools library. That is certainly where it belongs in a logical sense,
I'm not so sure about that. Mathematics deals with real numbers. Computer science deals with floating point numbers. Ask a typical mathematician what a NaN, denormal number or negative zero is, and you are most likely to get a blank stare. From a purely logical point of view it makes sense to have a separate floating point library, just as there is a separate integer library. A more pragmatic reason to keep the floating point functions together with the proper math functions (exponential functions, trig functions, special functions, statistical distributions etc.) is that in TR1 both are declared in the header <cmath>. --Johan
On Tue, Feb 26, 2008 at 12:47 PM, Johan Råde <rade@maths.lth.se> wrote:
John Phillips wrote:
I am also concerned about merging it into the MathTools library. That is certainly where it belongs in a logical sense,
I'm not so sure about that. Mathematics deals with real numbers. Computer science deals with floating point numbers.
There is a whole branch of mathematics (Numerical Analysis) that deals w/ numeric representation, and I don't know any mathematicians who aren't at least aware of it. Jon
Jonathan Franklin wrote:
On Tue, Feb 26, 2008 at 12:47 PM, Johan Råde <rade@maths.lth.se> wrote:
John Phillips wrote:
I am also concerned about merging it into the MathTools library. That is certainly where it belongs in a logical sense, I'm not so sure about that. Mathematics deals with real numbers. Computer science deals with floating point numbers.
There is a whole branch of mathematics (Numerical Analysis) that deals w/ numeric representation, and I don't know any mathematicians who aren't at least aware of it.
The binary representation of floating point numbers is computer science, not mathematics. Numerical analysis is an interdisciplinary field, involving both mathematics and computer science. --Johan
On Tue, Feb 26, 2008 at 1:32 PM, Johan Råde <rade@maths.lth.se> wrote:
The binary representation of floating point numbers is computer science, not mathematics. Numerical analysis is an interdisciplinary field, involving both mathematics and computer science.
The field of numerical analysis predates the invention of modern computers (and hence CS) by many centuries. It's true that Numerical Analysis deals w/ more than just binary representation of real numbers, but it *does* deal w/ them. Speaking from my experience, I know more Mathematicians who understand how floating point numbers work than I do computer scientists who know, and I probably know 10 computer scientists for each mathematician that I know. I am always dismayed by how many computer scientists don't really know anything about floating point numbers, other than the obvious. This may be related to more Math departments requiring a numerical analysis class than CS departments, but I don't really know. Take that for whatever it's worth. Your experience may be very different from mine. Jon
Don't you think a Bessel function is somehow more "mathematical" than the isnormal function? --Johan
Johan Råde wrote:
Don't you think a Bessel function is somehow more "mathematical" than the isnormal function?
--Johan
No, but that may be a function of my personal view of mathemmatics. isnormal is a set classification function, and as such has a long and important history in mathematics. It isn't what most non-mathematicians think of when they consider mathematical functions, but many very important mathematical advances that happened before the advent of computers were based around such classification functions. isnormal in specific isn't a classification rule that I expect would have been used before digital computers, but the underlying model of functions that trace what is and is not included in a set is an old one in mathematics. John
On Tue, Feb 26, 2008 at 2:09 PM, John Phillips < phillips@delos.mps.ohio-state.edu> wrote:
Johan Råde wrote:
Don't you think a Bessel function is somehow more "mathematical" than the isnormal function?
--Johan
No, but that may be a function of my personal view of mathemmatics. isnormal is a set classification function, and as such has a long and important history in mathematics. It isn't what most non-mathematicians think of when they consider mathematical functions, but many very important mathematical advances that happened before the advent of computers were based around such classification functions. isnormal in specific isn't a classification rule that I expect would have been used before digital computers, but the underlying model of functions that trace what is and is not included in a set is an old one in mathematics.
Excellent points, though I'll refrain from making any judgement calls as to what is more "mathematical". I know too many different flavors of mathematician. Is Analysis more "mathematical" than Combinatorics, or <insert your mathematical discipline here>? ;-) Jon
On Tue, Feb 26, 2008 at 1:32 PM, Johan Råde <rade@maths.lth.se> wrote:
The binary representation of floating point numbers is computer science, not mathematics. Numerical analysis is an interdisciplinary field, involving both mathematics and computer science.
Note that Indian mathematicians were using floating point numbers in the 14th century: http://en.wikipedia.org/wiki/Kerala_School Binary representations would of course not be nearly as interesting until the advent of computers... but number systems (including binary) are a part of Mathematics. Some might even argue that (theoretical) Computer Science is itself a branch (or at least red-headed step-child) of Mathematics. ;-) Jon
Johan Råde wrote:
John Phillips wrote:
I am also concerned about merging it into the MathTools library. That is certainly where it belongs in a logical sense,
I'm not so sure about that. Mathematics deals with real numbers. Computer science deals with floating point numbers. Ask a typical mathematician what a NaN, denormal number or negative zero is, and you are most likely to get a blank stare.
From a purely logical point of view it makes sense to have a separate floating point library, just as there is a separate integer library.
A more pragmatic reason to keep the floating point functions together with the proper math functions (exponential functions, trig functions, special functions, statistical distributions etc.) is that in TR1 both are declared in the header <cmath>.
--Johan
Sorry for miss-naming and calling the Math Toolkit by the incorrect MathTools, but the basis for my concern that they will be merged is from the review announcement, where John Maddock says: John Maddock wrote:
1) Floating point classification routines: these are optimised implementations of the C99 and C++ TR1 functions fpclassify, isinf, isnan, isnormal and isfinite. From Boost-1.35 onwards these are already a part of Boost.Math (see http://svn.boost.org/svn/boost/trunk/libs/math/doc/sf_and_dist/html/math_too...)
so if accepted the two implementations will get merged.
The review here should focus on the implementation used, and testing on whatever platforms you have available - in particular are there any circumstances (compiler optimisation settings etc) where this implementation breaks?
2) Sign manipulation functions: implementations of the C99 and C++ TR1 functions copysign and signbit, plus the changesign function. Two of these (signbit and copysign) are currently undocumented members of Boost.Math, and again the two implementations will get merged if this library is accepted.
Again the main focus of the review here is the implementation, and testing thereof especially in the presence of compiler optimisations.
As for typical mathematicians and NANs, denorms and such, I would tend to disagree with you. My numerical analysis books certainly include these ideas (Starting with a book by Hamming in 1973, at least.), and even math programs that don't require that class strongly encourage it (The one I teach in requires it, and these concept are taught in it.). However, all of that misses my concern. I think the library you are providing is important and useful for anyone who wants to do computational numerics well, and I want to make sure that it is not lost for the casual reader of the boost documentation. If it gets merged into the Math Toolkit documentation as the notes above imply to me (and I'm sorry if I'm miss interpreting them), then I'm afraid that many users of boost tools will miss both the significance and utility of your work. I would just like to make sure that if such a merger happens, care is taken to highlight the utilities properly. John
John Phillips wrote:
Sorry for miss-naming and calling the Math Toolkit by the incorrect MathTools,
No worries :-)
but the basis for my concern that they will be merged is from the review announcement, where John Maddock says:
The intention (at least on my part) is to merge the implementations so that we don't end up with two different versions(!), and so that we get the best of both worlds: the genericity of the current Boost.Math version (works to a degree with numbers represented as class types), and the performance of Johan's version for the builtin floating point types. How the library and documentation should be structured is a whole other issue - thanks for raising this - I think you're correct that these utilities should be easy to find and at the very least flagged up on the library index page. I'm open to suggestions as to whether that means separate documentation and organisation, or just a separate index entry leading to the sub-TOC for fpclassify etc. There are pro's and cons either way, so I'm easy I guess. John.
John Maddock wrote:
John Phillips wrote:
Sorry for miss-naming and calling the Math Toolkit by the incorrect MathTools,
No worries :-)
but the basis for my concern that they will be merged is from the review announcement, where John Maddock says:
The intention (at least on my part) is to merge the implementations so that we don't end up with two different versions(!), and so that we get the best of both worlds: the genericity of the current Boost.Math version (works to a degree with numbers represented as class types), and the performance of Johan's version for the builtin floating point types.
How the library and documentation should be structured is a whole other issue - thanks for raising this - I think you're correct that these utilities should be easy to find and at the very least flagged up on the library index page.
I'm open to suggestions as to whether that means separate documentation and organisation, or just a separate index entry leading to the sub-TOC for fpclassify etc. There are pro's and cons either way, so I'm easy I guess.
John.
I agree with John. I think the floating point classification and signbitfunctions should be made a subentry of Boost.Math but not of Boost.Math.SpecialFunctions. "Special functions" is a mathematical term, that, although not precisely defined, usually refers to certain classes of analytical functions. The phrase "Special functions" as used by mathematicians do not include floating point classification functions. Where should the facets go? The same place? --Johan
OK, finally(!), here comes my review of this library - purely as a potential user, not as review manager. First off the headline: I think the library should be accepted into Boost. fpclassify ~~~~~~~~~~ Having already demonstated my ignorance of the aliasing rules, I'm quite happy to accept this as is :-) There's little to say design wise, as it follows existing practice, so as long as Johan is happy to see this merged with my code that handles non-builtin floating point numbers (class types like NTL::RR or mpfr_class etc) then I'm happy. It's been suggested that isinf should indicate the sign of the infinity. Personally I would prefer isinf etc to return a bool as per TR1 than return +1 or -1 etc to indicate the sign as well (as per glibc) - we have other functions for that purpose, and IMO a function name should indicate it's purpose clearly - relying on the sign of the result of isinf seems like a recipe for hard to maintain code to me... just my 2c worth. Sign Manipulation ~~~~~~~~~~~~~~~~~ As these use the same core code as the fpclassify code, I'm happy with the implementation. We've discussed changesign previously, so I won't labour the point again :-) There is one other useful function worth having IMO: template <class T> int sign(T x); returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time. Number Facets ~~~~~~~~~~~~~ I think these are a good idea, a few comments about the interface follow: 1) I assume that these: const int legacy; const int signed_zero; const int trap_infinity; const int trap_nan; Should be declared "static" as well? 2) I'm not completely sure about the "legacy" flag: shouldn't the facet just read in everything that it recognises by default? Or maybe we should flip the meanings and "legacy" should become "strict_read" or "nolegacy" or something - I'm not that keen on those names, but Boosters can usually come up with something snappy when asked :-) 3) I think someone else mentioned that there is a lot of boilerplate code that gets repeated when using these facets, so I think I'd like to see something like: std::locale get_nonfinite_num_locale(int flags = 0, std::locale base_locale = std::locale()); which returns a locale containing the same instantiations of these facets that a default locale would have for num_put and num_get, which is to say: nonfinite_num_get<char>, nonfinite_num_get<wchar_t> nonfinite_num_put<char> nonfinite_num_get<wchar_t> Then we could simply write: my_iostream.imbue(get_nonfinite_num_locale()); or std::locale::global(get_nonfinite_num_locale()); and be done with it. 4) I think the flags are useful, but would be a lot nicer as iostream manipulators - although I accept this may not work with lexical_cast :-( BTW what was the issue that would require separate file compilation for manipulators? Overall nice job. Regards, John.
John Maddock wrote:
It's been suggested that isinf should indicate the sign of the infinity. Personally I would prefer isinf etc to return a bool as per TR1 than return +1 or -1 etc to indicate the sign as well (as per glibc) - we have other functions for that purpose, and IMO a function name should indicate it's purpose clearly - relying on the sign of the result of isinf seems like a recipe for hard to maintain code to me... just my 2c worth.
I agree.
Sign Manipulation ~~~~~~~~~~~~~~~~~
As these use the same core code as the fpclassify code, I'm happy with the implementation. We've discussed changesign previously, so I won't labour the point again :-)
I will remove changesign from the library.
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
I'd be happy to add that.
Number Facets ~~~~~~~~~~~~~
I think these are a good idea, a few comments about the interface follow:
1) I assume that these:
const int legacy; const int signed_zero; const int trap_infinity; const int trap_nan;
Should be declared "static" as well?
Stupid me.
2) I'm not completely sure about the "legacy" flag: shouldn't the facet just read in everything that it recognises by default? Or maybe we should flip the meanings and "legacy" should become "strict_read" or "nolegacy" or something - I'm not that keen on those names, but Boosters can usually come up with something snappy when asked :-)
There are cases where the legacy flag could change the meaning of a file. For instance, if you set the legacy flag, and parse the string "1#2", then "1#" will be interpreted as the beginning of "1#INF" or some other Visual Studio nonsense. Once the parser gets to '2', it will realize its mistake, but when you read using stream iterators it is impossible to backtrack, so the parser will set the failbit of the stream. So the legacy flag can be a little dangerous. This should of course be pointed out in the docs.
3) I think someone else mentioned that there is a lot of boilerplate code that gets repeated when using these facets, so I think I'd like to see something like:
std::locale get_nonfinite_num_locale(int flags = 0, std::locale base_locale = std::locale());
which returns a locale containing the same instantiations of these facets that a default locale would have for num_put and num_get, which is to say:
nonfinite_num_get<char>, nonfinite_num_get<wchar_t> nonfinite_num_put<char> nonfinite_num_get<wchar_t>
Then we could simply write:
my_iostream.imbue(get_nonfinite_num_locale());
or
std::locale::global(get_nonfinite_num_locale());
and be done with it.
Since I now have received two requests for this feature, I will add it.
4) I think the flags are useful, but would be a lot nicer as iostream manipulators - although I accept this may not work with lexical_cast :-(
BTW what was the issue that would require separate file compilation for manipulators?
It is about four years since last time I defined my own manipulators, but this is how I remember it: When you define a manipulator you need to store some state information with the stream. Streams contain an integer array that should be used for that purpose. The nonfinite_num_facets would need to reserve a position in those arrays. That is done by calling std::xalloc, exactly once, and storing the returned integer value in a global integer variable. You must make sure that there is exactly one instance of that variable. If you pass stream objects between different DLL's, then you must make sure that all the DLL's share the same instance of that variable. Can that be guaranteed without having a cpp file, and compiling that file as a DLL that is used by the other DLL's? Any comments?
Overall nice job.
Regards, John.
Thank you, Johan
AMDG Johan Råde wrote:
Number Facets ~~~~~~~~~~~~~
I think these are a good idea, a few comments about the interface follow:
1) I assume that these:
const int legacy; const int signed_zero; const int trap_infinity; const int trap_nan;
Should be declared "static" as well?
Stupid me
Actually, they are implicitly static anyway. (7.1.1/6) In Christ, Steven Watanabe
Johan Råde wrote:
2) I'm not completely sure about the "legacy" flag: shouldn't the facet just read in everything that it recognises by default? Or maybe we should flip the meanings and "legacy" should become "strict_read" or "nolegacy" or something - I'm not that keen on those names, but Boosters can usually come up with something snappy when asked :-)
There are cases where the legacy flag could change the meaning of a file. For instance, if you set the legacy flag, and parse the string "1#2", then "1#" will be interpreted as the beginning of "1#INF" or some other Visual Studio nonsense. Once the parser gets to '2', it will realize its mistake, but when you read using stream iterators it is impossible to backtrack, so the parser will set the failbit of the stream.
So the legacy flag can be a little dangerous. This should of course be pointed out in the docs.
Ah, I see, in that case it's probably best to leave as is, and add an "admonishment" to the docs to point this out.
4) I think the flags are useful, but would be a lot nicer as iostream manipulators - although I accept this may not work with lexical_cast :-(
BTW what was the issue that would require separate file compilation for manipulators?
It is about four years since last time I defined my own manipulators, but this is how I remember it:
When you define a manipulator you need to store some state information with the stream. Streams contain an integer array that should be used for that purpose. The nonfinite_num_facets would need to reserve a position in those arrays. That is done by calling std::xalloc, exactly once, and storing the returned integer value in a global integer variable. You must make sure that there is exactly one instance of that variable. If you pass stream objects between different DLL's, then you must make sure that all the DLL's share the same instance of that variable. Can that be guaranteed without having a cpp file, and compiling that file as a DLL that is used by the other DLL's?
This is tricky, the usual way would be to use boost::call_once: template <bool dummy> struct iostream_data { private: static int& get_iword_index() { static int index; return index; } static void init() { get_iword_index() = std::ios_base::xalloc(); } public: static void index() { static boost::once_flag f = BOOST_ONCE_INIT; boost::call_once(&iostream_data<dummy>::init, f); return get_iword_index(); } }; Then the manipulator can use iostream_data<true>::index() to obtain the index of iword it wants to use. The problem as you correctly point out is with DLL's: I'm fairly sure that on Unix (or Linux anyway) the shared library loader will resolve duplicate symbols so that just one is active in the program - that way only one instance of iostream_data<dummy> will be active and everything should work OK. On Windows though you could end up with separate instances of iostream_data<dummy> in each dll, which would be bad news if you expect to pass iostreams between them :-( The only - somewhat inelegant - solution I can think of is to define a pair of macros to import/export iostream_data<dummy> so users can control it's instantiation. Cheers, John.
John Maddock wrote:
2) I'm not completely sure about the "legacy" flag: shouldn't the facet just read in everything that it recognises by default? Or maybe we should flip the meanings and "legacy" should become "strict_read" or "nolegacy" or something - I'm not that keen on those names, but Boosters can usually come up with something snappy when asked :-) There are cases where the legacy flag could change the meaning of a file. For instance, if you set the legacy flag, and parse the string "1#2", then "1#" will be interpreted as the beginning of "1#INF" or some other Visual Studio nonsense. Once the parser gets to '2', it will realize its mistake, but when you read using stream iterators it is impossible to backtrack, so the parser will set the failbit of the stream.
So the legacy flag can be a little dangerous. This should of course be pointed out in the docs.
Ah, I see, in that case it's probably best to leave as is, and add an "admonishment" to the docs to point this out.
One could have support for reading "qnan", "nanq", "snan" and "nans" as default. (The scenarios where that could change the meaning of a valid file are extremely contrived.) And have a flag for enabling reading of the Visual Studio nonsense.
The problem as you correctly point out is with DLL's: I'm fairly sure that on Unix (or Linux anyway) the shared library loader will resolve duplicate symbols so that just one is active in the program - that way only one instance of iostream_data<dummy> will be active and everything should work OK. On Windows though you could end up with separate instances of iostream_data<dummy> in each dll, which would be bad news if you expect to pass iostreams between them :-( The only - somewhat inelegant - solution I can think of is to define a pair of macros to import/export iostream_data<dummy> so users can control it's instantiation.
How do other Boost libraries handle this problem? --Johan
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
What should this function return for a NaN? --Johan
Quoting Johan Råde <rade@maths.lth.se>:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
What should this function return for a NaN?
A NaN?
----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Johan Råde Sent: 27 February 2008 15:33 To: boost@lists.boost.org Subject: Re: [boost] Ann: Floating Point Utilities Review starts today
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
What should this function return for a NaN?
well, using INT_MIN would preserve the identify sign(-x) == -sign(x) even for x=NAN on many platforms. Note: this is not an entirely serious suggestion. Seriously, I would have thought either "unspecified" or throw or pick a value. I think it's probably an error to call this with a NAN. From: Peter Bartlett
A NaN?
Tricky. sign was specified as returning "int" (and I think that's probably right). -- Martin Bonner Senior Software Engineer/Team Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer
Johan Råde wrote:
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
What should this function return for a NaN?
In any of the use cases I have for that function, it would be an error to be trying to handle NaN's. So it's a domain error I guess. John.
John Maddock wrote:
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
The library deals with signbits, not with signs. So I think that adding this function to the library would just create confusion. It may be a useful function, but I don't think it belongs here. --Johan
Johan Råde wrote:
John Maddock wrote:
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
The library deals with signbits, not with signs.
Nod, understood.
So I think that adding this function to the library would just create confusion. It may be a useful function, but I don't think it belongs here.
Maybe, maybe not. I was just thinking to locate all the sign-related functions in one place rather than scatter them round the place, but it can always remain an implementation detail if it doesn't fit in well with the rest. John.
On Wed, Feb 27, 2008 at 10:56 AM, Johan Råde <rade@maths.lth.se> wrote:
John Maddock wrote:
There is one other useful function worth having IMO:
template <class T> int sign(T x);
returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0). This is trivial to implement (lot's of old C code - for example the Numerical Recipies stuff - implement this as a helper macro), and there's an undocumented version currently in Boost.Math. Is this worth integrating with Johan's code? I've found it quite useful in Boost.Math from time to time.
The library deals with signbits, not with signs. So I think that adding this function to the library would just create confusion. It may be a useful function, but I don't think it belongs here.
--Johan
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
I agree. Steve
participants (15)
-
Giovanni Piero Deretta -
Guillaume Melquiond -
Johan Råde -
John Maddock -
John Phillips -
Jonathan Franklin -
Martin Bonner -
Neal Becker -
Paul A Bristow -
Peter Bartlett -
Sebastian Redl -
Stephan Tolksdorf -
Steve Ward -
Steven Watanabe -
Zach Laine