search

dftlibs / xcfun

XCFun: A library of exchange-correlation functionals with arbitrary-order derivatives
https://dftlibs.org/xcfun/
Mozilla Public License 2.0
57 stars 32 forks source link

Fixed clang error: call to 'cbrt' is ambiguous #27

Closed yurivict closed 7 years ago

yurivict commented 7 years ago

This resolves https://github.com/dftlibs/xcfun/issues/22

robertodr commented 7 years ago

Thank you! The continuous integration builds on Travis are currently failing for reasons unrelated to your PR. Let's wait until @uekstrom and/or other developers can have a say.

uekstrom commented 7 years ago

The "correct" way would be to figure out what numeric type (T=double etc) libtaylor uses, and do cbrt(T(2)) to get the cube root with the right precision. But I support this commit because it is consistent with other constants in the code, which are in double precision.

robertodr commented 7 years ago

Thanks Ulf for weighing in. I have two questions:

  1. Should all similar instances in XCFun be tracked down and corrected in a similar way?
  2. Just out of curiosity, is there a way to implement the correct approach?
uekstrom commented 7 years ago

It is not so easy to do cleanly, see how to extend numeric_limits in c++ with your own numeric type. Using double is almost always the right choice.

Ulf

On Tue, 13 Jun 2017 at 18:47, Roberto Di Remigio notifications@github.com wrote:

Thanks Ulf for weighing in. I have two questions:

  1. Should all similar instances in XCFun be tracked down and corrected in a similar way?
  2. Just out of curiosity, is there a way to implement the correct approach?

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/dftlibs/xcfun/pull/27#issuecomment-308178251, or mute the thread https://github.com/notifications/unsubscribe-auth/AMeumXa9OWvysz_yUusP4ahLvtaIh6DRks5sDr0GgaJpZM4N4uTS .

robertodr commented 7 years ago

Would a construct using std::is_same do the trick? It's C++11 though...

uekstrom commented 7 years ago

It's not immediate clear to me how the code would look. The problem is that the return value of sqrt(x) is based on the type of x, not on the type assigned to. We have the same problem with constants such as pi. C solved this nicely in the "C world" by using doubles, but I don't know if there is a general solution in C++. The QD high precision library struggles with this issues as well. Of course anything is possible if you accept ugly code, but the formulas a difficult enough to read as it is. C++11 is no problem, I wish I had time to rewrite the whole library in more modern C++.