Use fp_traits_non_native directly in eos-portable-archive
#40057
Conversation
In the eos-portable-archive sources that were copy-pasted into `CondFormats/Serialization`, the boost implementation detail `fp_traits<T>::value` is used, which is expected to resolve to `fp_traits_non_native` that has the `bits` and `set_bits` attributes, which are used in the code. But depending on the compiler flags, `fp_traits<T>::value` can also resolve to `fp_traits_native`, which doesn't have `bits` and `set_bits`. It only works when `BOOST_MATH_DISABLE_STD_FPCLASSIFY` is set. To make the CMSSW code compile independent of the boost flags, this commit suggests to directly use `fp_traits_non_native` in exactly the same way as it is used inside `fp_traits` in the `boost/math/special_functions/detail/fp_traits.hpp` header file from boost. See also: daldegam/eos-portable-archive#5
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40057/33032
|
|
A new Pull Request was created by @guitargeek (Jonas Rembser) for master. It involves the following packages:
@malbouis, @cmsbuild, @saumyaphor4252, @ggovi, @francescobrivio, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
| * integral type that was stored in the stream. Francois Mauger provided | ||
| * standardized behaviour for special values like inf and NaN, that need to | ||
| * be serialized in his application. | ||
| * | ||
| * \note by Johan Rade (author of the floating point utilities library): | ||
| * Be warned that the math::detail::fp_traits<T>::type::get_bits() function | ||
| * Be warned that the math::detail::fp_traits_non_native<T,U>::get_bits() function |
There was a problem hiding this comment.
hi @guitargeek is this comment still true with fp_traits_non_native ?
There was a problem hiding this comment.
I think so, yes, because before fp_traits<T> was resolving to fp_traits_non_native::type anyway, so is must come from there (fp_traits_non_native is the only fp_traits implementation that has a get_bits member, fp_traits_native doesn't have it)
|
@cmsbuild , please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a474d1/29021/summary.html Comparison SummarySummary:
|
|
OK, if I understand correctly the unit tests |
|
Good question, I'm not sure which tests cover this change. I just did whatever was necessary to make the code compile without the |
|
Hi @ggovi! Can you please make a statement? It would be nice to merge this PR at some point to make the CMSSW code more portable. |
|
+db
|
cmsbuild
added
db-approved
fully-signed
and removed
db-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
|
Hello, my understanding is that the unit tests listed above should involve the change, in particular So, it looks ok with me. |
|
Thank you! |
In the eos-portable-archive sources that were copy-pasted into
CondFormats/Serialization, the boost implementation detailfp_traits<T>::valueis used, which is expected to resolve tofp_traits_non_nativethat has thebitsandset_bitsattributes, which are used in the code.But depending on the compiler flags,
fp_traits<T>::valuecan also resolve tofp_traits_native, which doesn't havebitsandset_bits. It only works whenBOOST_MATH_DISABLE_STD_FPCLASSIFYis set.To make the CMSSW code compile independent of the boost flags, this commit suggests to directly use
fp_traits_non_nativein exactly the same way as it is used insidefp_traitsin theboost/math/special_functions/detail/fp_traits.hppheader file from boost:https://github.com/boostorg/math/blob/master/include/boost/math/special_functions/detail/fp_traits.hpp#L570
This PR is part of my ongoing effort of making CMSSW compile on other platforms more easily.
See also:
daldegam/eos-portable-archive#5