Fully functional std::complex specializations and overloads

[ChrisChiasson]

@Bob_Carpenter

I’ll check out what’s going on. I may not have been testing that case.

I can try to fire up your PR and see if it passes all of my tests. I thought this was going to be a lot easier than it is.

It would be interesting to see what would happen testing my PR. In many ways, I envy the testing framework you guys have now, because of the finite difference checks and the different type instantiations. Back when I did my implementation, I was mortally afraid of results being wrong (especially given what I’m using it for), so I wrote as little code as possible to leave less surface area for errors.
TL;DR: In my day, we walked to school in the snow uphill both ways. We ate dirt, and we were thankful.

@Bob_Carpenter

I’ve pushed fixes so that the header tests now pass and pow(int, var) and pow(var, int) work.

The unit tests are still failing to compile in various places due to ambiguities.

Understood
I’ll be back in the office Monday & will try to check. Also, I may be slow to respond next week due to an industry meeting I have to attend. Apologies in advance.

[ChrisChiasson]

@Bob_Carpenter

The main difference is that I never had to take extreme measures to set things to zero. I just defined the constructors in std::complex<var> and std::complex<fvar<T>> to do that.

For me, because of the choice to delegate back to the standard library implementation, and because of g++ using _Tp() everywhere to represent zero, there were many functions where I needed zeroing: real assignment (239), most of the comparator ops (465, 470, 483, 488), abs (589), sqrt (894), pow(1021). Clang wasn’t nearly as bad, at least in that one respect.

[Bob_Carpenter]

@ChrisChiasson

Also, I may be slow to respond next week due to an industry meeting I have to attend. Apologies in advance.

No worries. I’m also traveling and haven’t been doing a lot around this. I’ll be back in the office next week myself.

[ChrisChiasson]

I updated to commit 0769d931462b861ae08cff6129d88b0d9b3556fa (HEAD -> feature/0123-complex-var, origin/feature/0123-complex-var). I get

./src/Nautical/RAO/RAO.hpp:80:78: error: no match for 'operator<<' (operand types are 'Eigen::Matrix<std::complex<stan::math::var>, 3, 3, 0, 3, 3>' and 'double')
   80 |   return (Eigen::Matrix<decltype(std::complex(f_*phi_x*theta_y*psi_z)),3,3>()<<
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
   81 |          0., -a_*psi_z, b_*theta_y,
      |          ~~

Update: ran it through clang++ instead of g++ above. The error is the same as g++, but clang dumped more information on the failed overloads. It looks like it cannot find a way to construct a complex<var> from a double. (Edit note: I had said this backwards earlier today)

../eigen-git-mirror\Eigen/src/Core/DenseBase.h:312:31: note: candidate function not viable: no known conversion from 'double' to 'const Eigen::DenseBase<Eigen::Matrix<std::complex<stan::math::var>, 3, 3, 0, 3, 3> >::Scalar' (aka 'const std::complex<stan::math::var>') for 1st argument
    CommaInitializer<Derived> operator<< (const Scalar& s);

[Bob_Carpenter]

I haven’t tried to overload for Matrix<T, 3, 3> as we only use dynamic matrices and vectors in Stan, not fixed size ones. I’ll see if I can get this to compile, but it’s not generally in scope for Stan. Very little of our matrix code is general enough to handle Matrix<T, 3, 3>.

[ChrisChiasson]

The far right of the clang message just says it can’t convert double to complex var, which usually means it can’t find the relevant ctor just for that conversion itself, ignoring what kind of matrix is in play.

[Bob_Carpenter]

I’ll try to get this building. Right now, I got a new notebook, upgraded clang++ and can’t get anything to build.

[Bob_Carpenter]

@mitzimorris figured out how to configure the new clang++ on High Sierra. Thanks! She said she’ll post what she did on Discourse. Here’s my new clang version:

Apple clang version 11.0.0 (clang-1100.0.33.16)
Target: x86_64-apple-darwin19.2.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

I had to fix some merge conflicts with the include-what-you-use fixes and an ambiguity issue with the base constructor. Now the std_complex test is passing and I’ll run the rest of the tests and then start breaking down into a sequence of digestible pull requests.

[mitzimorris]

problems described here - best guess - old crap from macports - any of y’all please advise -

Upgrade to Catalina incurred problems building math lib dynamic archives (sundials)

Bob ran into problems compiling stan_math on his new MacBook pro - which he blogged about here: https://statmodeling.stat.columbia.edu/2019/12/11/macbook-pro-16-inch-2019-quick-review/ Ease of upgrade Apple makes it super easy to move everything from an old machine. Once I entered enough passwords on my menagerie of Apple devices, it took less than 2 hours to transfer everything from the old machine to the new one via my home wireless network. The only software I’ve had to upgrade to get ba…

[Bob_Carpenter]

Thanks, @ChrisChiasson. I had thought I had comprehensive constructor and assignment tests for the type combinatorics. Now I do. I fixed the failing ones by adding primitive constructor overloads to std::complex.

Now all of our math lib tests are passing other than multiple translation unit testing because a full class specialization can only be in one translation unit. So I have to factor the class std:complex<var> into declaration and definition files. I’ll do that when I stage the PRs for review.

[ChrisChiasson]

No problemo. I’ll continue trying to compile on Monday when I get back in the office. Would you like me to switch my reporting to the new thread?

[Bob_Carpenter]

Either way’s fine with me. I managed to get your tests to succeed by changing some of the meta calls to match the new metaprogram refactor and using value_of_rec.

Also managed to use Homebrew to install g++6 and g++9 and it works for those compilers, too.

I could not do simple things like remove my overloads for sqrt() in favor of templates because it wound up being ambiguous with the built-in library function. When I just removed the overloads, thinking the library function would work, it’s missing member variables that the library std::complex<T> instantiations require. I think this may be what they mean by “undefined” behavior when instantiating with non-primitive types.

Given that everything’s working here, I’m going to start chunking into pull requests for Stan. Code freeze for the next official Stan release will be some time in the first or second week of January.

[ChrisChiasson]

@Bob_Carpenter

I could not do simple things like remove my overloads for sqrt()

I think you’re already at the minimum number of function definitions for a unary function like sqrt. I see the one for var, and the one for fvar, both of which must exist in order to override an exact match for the existing template in the standard library (2nd caveat mentioned in other thread). In my implementation, I don’t even have a sqrt definition at all because it uses the one from the standard library as-is.