Help testing release candidate for std::complex

+1, sounds good to me. It would prevent the need to patch either patch eigen as above or experiment with different versions of the big ScalarBinaryOpTraits template.

Evertyhing’s working for all the var and fvar overloads of binary ops. That is, we can multiply arbitrary compatible double, var, fvar, and complex versions of the same.

@ChrisChiasson—with this latest push to the branch, is there anything else that you need?

For commit 97bd50fb9eea3c3c24beb73188ee6abc536da3d8 (HEAD -> feature/0123-complex-var, origin/feature/0123-complex-var), I get:

g++ -std=gnu++2a -pedantic -pedantic-errors -Wall -Wextra -Werror -Wno-overlength-strings -Wfatal-errors -ffunction-sections -fdata-sections -pthread -march=native -mtune=native -mno-avx -fmessage-length=0 -I./src -isystem../boost -isystem../eigen-git-mirror -isystem../new-stan-dev/stan/src -isystem../new-stan-dev/math -isystem../new-stan-dev/math/lib/sundials_4.1.0/include -isystem../new-stan-dev/math/lib/tbb_2019_U8/include -O3 -DNOMINMAX -D_REENTRANT -DNDEBUG  -c -MMD -o src/Math/Diff/DiffTest.o src/Math/Diff/DiffTest.cpp
In file included from ../new-stan-dev/math/stan/math/rev/core.hpp:49,
                 from ../new-stan-dev/math/stan/math/mix/meta.hpp:10,
                 from ../new-stan-dev/math/stan/math/mix.hpp:4,
                 from ./src/Math/Diff/Diff.hpp:7,
                 from src/Math/Diff/DiffTest.cpp:1:
../new-stan-dev/math/stan/math/rev/core/std_complex.hpp:971:8: error: extra qualification not allowed [-fpermissive]
  971 | struct Eigen::ScalarBinaryOpTraits<double, std::complex<stan::math::var>,
      |        ^~~~~
compilation terminated due to -Wfatal-errors.
make: *** [<builtin>: src/Math/Diff/DiffTest.o] Error 1

With clang, I get:

$ clang++ -std=gnu++2a -pedantic -pedantic-errors -Wall -Wextra -Werror -Wno-overlength-strings -Wfatal-errors -ffunction-sections -fdata-sections -pthread -march=native -mtune=native -mno-avx -fmessage-length=0 -I./src -isystem../boost -isystem../eigen-git-mirror -isystem../new-stan-dev/stan/src -isystem../new-stan-dev/math -isystem../new-stan-dev/math/lib/sundials_4.1.0/include -isystem../new-stan-dev/math/lib/tbb_2019_U8/include -O3 -DNOMINMAX -D_REENTRANT -DNDEBUG  -c -MMD -o src/Math/Diff/DiffTest.o src/Math/Diff/DiffTest.cpp
In file included from src/Math/Diff/DiffTest.cpp:1:
In file included from ./src\Math/Diff/Diff.hpp:7:
In file included from ../new-stan-dev/math\stan/math/mix.hpp:4:
In file included from ../new-stan-dev/math\stan/math/mix/meta.hpp:10:
In file included from ../new-stan-dev/math\stan/math/rev/core.hpp:16:
In file included from ../new-stan-dev/math\stan/math/rev/core/matrix_vari.hpp:4:
In file included from ../new-stan-dev/math\stan/math/rev/fun/Eigen_NumTraits.hpp:4:
In file included from ../new-stan-dev/math\stan/math/rev/meta.hpp:6:
In file included from ../new-stan-dev/math\stan/math/rev/meta/operands_and_partials.hpp:5:
In file included from ../new-stan-dev/math\stan/math/rev/core/precomputed_gradients.hpp:4:
../new-stan-dev/math\stan/math/prim/err/check_consistent_sizes.hpp:28:53: fatal error: call to 'size' is ambiguous
  size_t max_size = std::max(is_vector<T1>::value * size(x1),
                                                    ^~~~
../new-stan-dev/math\stan/math/rev/core/precomputed_gradients.hpp:59:5: note: in instantiation of function template specialization 'stan::math::check_consistent_sizes<std::vector<stan::math::var, std::allocator<stan::math::var> >, std::vector<double, std::allocator<double> > >' requested here
    check_consistent_sizes("precomputed_gradients_vari", "vars", vars,
    ^
C:\msys64\mingw64\include\c++\9.2.0\bits/range_access.h:242:5: note: candidate function [with _Container = std::vector<stan::math::var, std::allocator<stan::math::var> >]
    size(const _Container& __cont) noexcept(noexcept(__cont.size()))
    ^
../new-stan-dev/math\stan/math/prim/meta/size.hpp:27:15: note: candidate function [with T = std::vector<stan::math::var, std::allocator<stan::math::var> >, $1 = void, $2 = void]
inline size_t size(const T& m) {
              ^
1 error generated.

The size() function needs the namespace. So
size_t max_size = std::max(is_vector<T1>::value * size(x1),
should be
size_t max_size = std::max(is_vector<T1>::value * stan::math::size(x1),

The problem is that operand_and_partials also has a size function and the compiler finds this use ambigious.

There was a nasty merge conflict in stan/math/opencl/kernel_generator/binary_operation.hpp which has nothing to do with my branch. No idea where it came from. I’m going to go with the current develop version and hope this gets sorted out before I try to generate a PR for all this.

Yikes. It all worked in clang++ :-).

It’s now up to date with develop and works with g++ again after I:

  • reverted binary_operation.hpp to its state in develop (not sure how conflict arose, but now the branch matches the develop branch)
  • added stan::math:: qualification to size() (not sure that was necessary)
  • removed Eigen:: qualifications within Eigen namespace (absolutely necessary)

:-) I like to think I make up for my badness by updating: Home · stan-dev/math Wiki · GitHub

For commit 4c58d66a9d381e073648879c27635b151ea2b4f3 (HEAD -> feature/0123-complex-var, origin/feature/0123-complex-var), I get:

g++ -std=gnu++2a -pedantic -pedantic-errors -Wall -Wextra -Werror -Wno-overlength-strings -Wfatal-errors -ffunction-sections -fdata-sections -pthread -march=native -mtune=native -mno-avx -fmessage-length=0 -I./src -isystem../boost -isystem../eigen-git-mirror -isystem../new-stan-dev/stan/src -isystem../new-stan-dev/math -isystem../new-stan-dev/math/lib/sundials_5.1.0/include -isystem../new-stan-dev/math/lib/tbb_2019_U8/include -O3 -DNOMINMAX -D_REENTRANT -DNDEBUG  -c -MMD -o src/Math/Diff/DiffTest.o src/Math/Diff/DiffTest.cpp
In file included from ../new-stan-dev/math/stan/math/prim/err/check_consistent_sizes.hpp:5,
                 from ../new-stan-dev/math/stan/math/rev/core/precomputed_gradients.hpp:4,
                 from ../new-stan-dev/math/stan/math/rev/meta/operands_and_partials.hpp:5,
                 from ../new-stan-dev/math/stan/math/rev/meta.hpp:6,
                 from ../new-stan-dev/math/stan/math/rev/fun/Eigen_NumTraits.hpp:4,
                 from ../new-stan-dev/math/stan/math/rev/core/matrix_vari.hpp:4,
                 from ../new-stan-dev/math/stan/math/rev/core.hpp:16,
                 from ../new-stan-dev/math/stan/math/mix/meta.hpp:10,
                 from ../new-stan-dev/math/stan/math/mix.hpp:4,
                 from ./src/Math/Diff/Diff.hpp:7,
                 from src/Math/Diff/DiffTest.cpp:1:
../new-stan-dev/math/stan/math/prim/err/check_consistent_size.hpp: In instantiation of 'void stan::math::check_consistent_size(const char*, const char*, const T&, size_t) [with T = std::vector<stan::math::var>; size_t = long long unsigned int]':
../new-stan-dev/math/stan/math/prim/err/check_consistent_sizes.hpp:30:24:   required from 'void stan::math::check_consistent_sizes(const char*, const char*, const T1&, const char*, const T2&) [with T1 = std::vector<stan::math::var>; T2 = std::vector<double>]'
../new-stan-dev/math/stan/math/rev/core/precomputed_gradients.hpp:60:50:   required from here
../new-stan-dev/math/stan/math/prim/err/check_consistent_size.hpp:26:55: error: call of overloaded 'size(const std::vector<stan::math::var>&)' is ambiguous
   26 |       || (is_vector<T>::value && expected_size == size(x))) {
      |                                                   ~~~~^~~
compilation terminated due to -Wfatal-errors.

With clang I get:

$ clang++ -std=gnu++2a -pedantic -pedantic-errors -Wall -Wextra -Werror -Wno-overlength-strings -Wfatal-errors -ffunction-sections -fdata-sections -pthread -march=native -mtune=native -mno-avx -fmessage-length=0 -I./src -isystem../boost -isystem../eigen-git-mirror -isystem../new-stan-dev/stan/src -isystem../new-stan-dev/math -isystem../new-stan-dev/math/lib/sundials_5.1.0/include -isystem../new-stan-dev/math/lib/tbb_2019_U8/include -O3 -DNOMINMAX -D_REENTRANT -DNDEBUG  -c -MMD -o src/Math/Diff/DiffTest.o src/Math/Diff/DiffTest.cpp

In file included from src/Math/Diff/DiffTest.cpp:1:
In file included from ./src\Math/Diff/Diff.hpp:7:
In file included from ../new-stan-dev/math\stan/math/mix.hpp:4:
In file included from ../new-stan-dev/math\stan/math/mix/meta.hpp:10:
In file included from ../new-stan-dev/math\stan/math/rev/core.hpp:16:
In file included from ../new-stan-dev/math\stan/math/rev/core/matrix_vari.hpp:4:
In file included from ../new-stan-dev/math\stan/math/rev/fun/Eigen_NumTraits.hpp:4:
In file included from ../new-stan-dev/math\stan/math/rev/meta.hpp:6:
In file included from ../new-stan-dev/math\stan/math/rev/meta/operands_and_partials.hpp:5:
In file included from ../new-stan-dev/math\stan/math/rev/core/precomputed_gradients.hpp:4:
In file included from ../new-stan-dev/math\stan/math/prim/err/check_consistent_sizes.hpp:5:
../new-stan-dev/math\stan/math/prim/err/check_consistent_size.hpp:26:51: fatal error: call to 'size' is ambiguous
      || (is_vector<T>::value && expected_size == size(x))) {
                                                  ^~~~
../new-stan-dev/math\stan/math/prim/err/check_consistent_sizes.hpp:30:3: note: in instantiation of function template specialization 'stan::math::check_consistent_size<std::vector<stan::math::var, std::allocator<stan::math::var> > >' requested here
  check_consistent_size(function, name1, x1, max_size);
  ^
../new-stan-dev/math\stan/math/rev/core/precomputed_gradients.hpp:59:5: note: in instantiation of function template specialization 'stan::math::check_consistent_sizes<std::vector<stan::math::var, std::allocator<stan::math::var> >, std::vector<double, std::allocator<double> > >' requested here
    check_consistent_sizes("precomputed_gradients_vari", "vars", vars,
    ^
C:\msys64\mingw64\include\c++\9.2.0\bits/range_access.h:242:5: note: candidate function [with _Container = std::vector<stan::math::var, std::allocator<stan::math::var> >]
    size(const _Container& __cont) noexcept(noexcept(__cont.size()))
    ^
../new-stan-dev/math\stan/math/prim/meta/size.hpp:27:15: note: candidate function [with T = std::vector<stan::math::var, std::allocator<stan::math::var> >, $1 = void, $2 = void]
inline size_t size(const T& m) {
              ^
1 error generated.

OK. I hadn’t realized there was a check_consistent_size.hpp in addition to check_consistent_sizes.hpp. I pushed a fix for that file.

The ambiguity of the Stan meta size() is with a definition in a file range_access.h. There’s no ambiguity with the Xcode version of clang++ or homebrew versions of g++ on Mac OS X. Is there a chance that include is coming from your driver program somewhere or from the compiler (mingw64?)? Also, is the definition in range_access.h not namespace qualified?

I’m worried because I see 370 lines in the math lib in which an unqualified size() call appear. We could make a separate PR and qualify all those. I don’t think there’s any alternative definitions of size() we would want called there.

Its probably something related to the msys2 windows development environment which @ChrisChiasson is using (at least that is what I gather from the wiki). With the typical RTools installation that we also use on Jenkins there is no such ambiguity with size().

Do things fail for you on develop also @ChrisChiasson?

We need to look into this, as Rtools 4.0 is shipping with gcc 8 and msys2 and presumably that will be what we recommend once it goes out of beta.

To me it looks like the problem is the 2nd function template in this file:
https://mc-stan.org/math/d1/d10/size_8hpp_source.html
The 2nd size function template is defined for any type that isn’t a stan scalar. It looks like it is designed to work with both Eigen types and C++ containers.

There is a way to detect if something is an Eigen type, and it looks like stan has already implemented it as the template is_eigen. So, if the 2nd size function template is constrained to that, we could probably prevent conflicts with size for the new Eigen that is compatible with the range syntax.

However, that still leaves C++17 containers…

In C++17 (see example at end of this link): https://en.cppreference.com/w/cpp/iterator/size
(and presumably Ranges, given the name of the conflicting file in my error message). There is a free function called size that will be defined by any compliant compiler and available for most standard containers.

Thus, it looks like the 2nd function in the file should be conditionally defined for standard containers in C++ versions less than C++17, either via SFINAE or macro checks. Alternatively, if it is defined directly on the vector type (instead of on a generic container class), it will always override the new C++17 function because it would be a tighter match, and thus would not need to be conditionally defined.

Edit to Edit: I had edited in an extra note down here, but I had read the reference incorrectly.

Thanks. I’ll take a look at updating it. I got slammed with a bunch of other stuff in the interim, but hope to get to this this week. It remains the branch that never gets merged.

I added std::vector<T> and Eigen::Matrix<T, R, C> specializations.

I also dealt with the latest round of merge conflicts and also removed the first PR’s worth of code, which just got merged into develop (none of it is std::complex related).

I pulled in commit 4a791d7c6997780b4e8e2123d00082f38cfef92b (HEAD -> feature/issue-123-complex-numbers-with-vars, origin/feature/issue-123-complex-numbers-with-vars).

On my machine, which has Eigen 3.4 (I’ve been using the new features), and the stan math commit 4a791 (see paragraph above this)… both clang and g++ are throwing compile errors with Eigen’s get_factor when I test the types of dynamic size complex vs non-complex matrix multiplications we were working on earlier. This setup doesn’t use the patches I made for Eigen above to fix complex packet math on non-exactly matching underlying types. To me, this indicates that in Eigen 3.4, the trick about not including template parameters on BinayOp may not work anymore.

I will try to confirm Friday by setting up an alternative environment with Eigen 3.3 to try to see if it is the library version or if it is something else (like all that crack smoking I’ve been doing recently).

I tried the suggested fixes for size(), which is in commit 4857c338aebdd628efbbc64ca2c2d07bcd667af2.

How did you get Eigen 3.4? I can’t figure out how to do it from their site. The page on 3.4 just points to the main page for downloads, but I don’t see anything there for 3.4. I also couldn’t find a relevant tag on the Eigen gitlab page.

‘3.4’ is just the default branch of their repo. They keep 3.3 with really high version numbers until ‘3.4’ is out of beta, then their default branch magically becomes 3.4.

I think I might have changed to the wrong directory before I executed git log (this is the command I use to copy out the value and name of the branch pointer). I think I changed to ~/stan-dev/math instead of ~/new-stan-dev/math, thus explaining why I have the wrong branch name outlined in my last post. Sorry I screwed that up.

My ~/new-stan-dev/math directory is on commit 4857c338aebdd628efbbc64ca2c2d07bcd667af2 (HEAD -> feature/0123-complex-var, origin/feature/0123-complex-var) (and was on it when I made the post above – verified by scrolling wayyyy up in MSYS2 and also by recompiling with g++ and clang and hitting same error).

Using the latest from the Eigen gitlab repo, I found that

struct general_matrix_matrix_product

in

rev/fun/Eigen_NumTraits.hpp

no longer worked because the number of template parameters and argument to the run() method changed. I just commented out the specialization and all the tests pass for me with both:

$ clang++ --version
Apple clang version 11.0.0 (clang-1100.0.33.16)

and

$ g++-9 --version
g++-9 (Homebrew GCC 9.2.0_2) 9.2.0

The specialization can be kept if modified as per the notes on that Eigen 3.4 thread I posted (would be ironic if that is what causes it to fail). I’ll try to get Eigen 3.3 running on Friday (actually, first, I’ll pull the latest Eigen).

edit to note: by “that” I mean, my notes modification to the specialization… (i.e. ironic if the failure were due to my own modification)

another edit: Horrifying thought. Packet math seems like the type of thing that may get conditionally triggered by platform or header versions. Does anyone listening have windows with msys2 and a recent g++ toolchain? ‘Full’ instructions (as full as I know how to make them, anyway) for that are on the math repo wiki under windows development notes.

Yeah, we have that in my office. I haven’t run this branch on it, but I was surprised (about a year ago) that we didn’t run into any new problems when compiling or running Stan models.

1 Like

Yes, I figured it would be possible. I just thought it’d be easier to edit it out for testing signatures. We’ll have to update it when they release 3.4 and we make the switch.

On 39f9c6af8a72037c030bbad998f7261417a6ccde (HEAD -> feature/0123-complex-var, origin/feature/0123-complex-var) I have switched to just trying to work with stan and the new complex tests to make it easier for us to test the same things (these commands are what the makefile generates, so it uses stan’s included eigen 3.3).

g++ and clang are erroring out with the same errors on each test, so I have just posted the clang version (created by copying the gcc version, typing ‘clan’, and then pasting).

Complex:

$ clang++ -std=c++1y -w -m64 -D_REENTRANT -Wall -Wno-unused-function -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-variable -Wno-sign-compare -Wno-unused-local-typedefs      -I lib/tbb_2019_U8/include -O3  -I . -I lib/eigen_3.3.3 -I lib/boost_1.72.0 -I lib/sundials_5.1.0/include -I lib/gtest_1.8.1/include -I lib/gtest_1.8.1 -I lib/gtest_1.8.1/include -I lib/gtest_1.8.1    -D_USE_MATH_DEFINES  -DBOOST_DISABLE_ASSERTS        -c -o test/unit/math/rev/core/std_complex_test.o test/unit/math/rev/core/std_complex_test.cpp
In file included from test/unit/math/rev/core/std_complex_test.cpp:1:
In file included from .\test/unit/math/test_ad.hpp:5:
In file included from .\test/unit/math/is_finite.hpp:4:
In file included from .\stan/math.hpp:148:
In file included from .\stan/math/rev.hpp:4:
In file included from .\stan/math/prim/fun/Eigen.hpp:13:
In file included from lib/eigen_3.3.3\Eigen/Dense:1:
In file included from lib/eigen_3.3.3\Eigen/Core:345:
lib/eigen_3.3.3\Eigen/src/Core/util/Meta.h:146:30: error: conversion from 'long long' to 'const std::complex<stan::math::var>' is ambiguous
  enum { value = sizeof(test(ms_from, 0))==sizeof(yes) };
                             ^~~~~~~
lib/eigen_3.3.3\Eigen/src/Core/util/Meta.h:155:18: note: in instantiation of template class 'Eigen::internal::is_convertible_impl<long long, std::complex<stan::math::var> >' requested here
  enum { value = is_convertible_impl<typename remove_all<From>::type,
                 ^
lib/eigen_3.3.3\Eigen/src/Core/PlainObjectBase.h:768:124: note: in instantiation of template class 'Eigen::internal::is_convertible<long long, std::complex<stan::math::var> >' requested here
    EIGEN_STRONG_INLINE void _init1(Index size, typename internal::enable_if<    (Base::SizeAtCompileTime!=1 || !internal::is_convertible<T, Scalar>::value)
                                                                                                                           ^
lib/eigen_3.3.3\Eigen/src/Core/Matrix.h:296:22: note: while substituting explicitly-specified template arguments into function template '_init1'
      Base::template _init1<T>(x);
                     ^
lib/eigen_3.3.3\Eigen/src/Eigenvalues/EigenSolver.h:149:9: note: in instantiation of function template specialization 'Eigen::Matrix<std::complex<stan::math::var>, -1, 1, 0, -1, 1>::Matrix<long long>' requested here
        m_eivalues(matrix.cols()),
        ^
test/unit/math/rev/core/std_complex_test.cpp:1481:34: note: in instantiation of function template specialization 'Eigen::EigenSolver<Eigen::Matrix<stan::math::var, -1, -1, 0, -1, -1> >::EigenSolver<Eigen::Matrix<stan::math::var, -1, -1, 0, -1, -1> >' requested here
  Eigen::EigenSolver<matrix_v_t> s(a);
                                 ^
test/unit/math/rev/core/std_complex_test.cpp:1494:3: note: in instantiation of function template specialization 'expectEigenSolver<stan::math::var>' requested here
  expectEigenSolver<var_t>();
  ^
.\stan/math/rev/core/std_complex.hpp:382:3: note: candidate constructor
  complex(float x)  // NOLINT(runtime/explicit)
  ^
.\stan/math/rev/core/std_complex.hpp:385:3: note: candidate constructor
  complex(double x)  // NOLINT(runtime/explicit)
  ^
.\stan/math/rev/core/std_complex.hpp:388:3: note: candidate constructor
  complex(long double x)  // NOLINT(runtime/explicit)
  ^
.\stan/math/rev/core/std_complex.hpp:391:3: note: candidate constructor
  complex(short int x)  // NOLINT
  ^
.\stan/math/rev/core/std_complex.hpp:394:3: note: candidate constructor
  complex(int x)  // NOLINT(runtime/explicit)
  ^
.\stan/math/rev/core/std_complex.hpp:397:3: note: candidate constructor
  complex(long int x)  // NOLINT
  ^

Complex 2:

$ clang++ -std=c++1y -w -m64 -D_REENTRANT -Wall -Wno-unused-function -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-variable -Wno-sign-compare -Wno-unused-local-typedefs      -I lib/tbb_2019_U8/include -O3  -I . -I lib/eigen_3.3.3 -I lib/boost_1.72.0 -I lib/sundials_5.1.0/include -I lib/gtest_1.8.1/include -I lib/gtest_1.8.1 -I lib/gtest_1.8.1/include -I lib/gtest_1.8.1    -D_USE_MATH_DEFINES  -DBOOST_DISABLE_ASSERTS        -c -o test/unit/math/rev/core/std_complex_test.o test/unit/math/rev/core/std_complex2_test.cpp
test/unit/math/rev/core/std_complex2_test.cpp:2:10: fatal error: 'stan/math/mix/mat.hpp' file not found
#include <stan/math/mix/mat.hpp>
         ^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Edit to add: I did add -w to the makefile, because the warnings are voluminous, obscuring the errors.

chris.chiasson@5CD9253ZG0 MINGW64 ~/new-stan-dev/math
$ git diff
diff --git a/make/compiler_flags b/make/compiler_flags
index 929647ee77..6f8c24c378 100644
--- a/make/compiler_flags
+++ b/make/compiler_flags
@@ -83,7 +83,7 @@ CPPFLAGS_GTEST ?=


 ## setup compiler flags
-CXXFLAGS_LANG ?= -std=c++1y
+CXXFLAGS_LANG ?= -std=c++1y -w
 #CXXFLAGS_BOOST
 CXXFLAGS_SUNDIALS ?= -pipe
 #CXXFLAGS_GTEST