New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 1589 - std::complex in GeneralizedEigenSolver prevents usage of custom scalar types.
Summary: std::complex in GeneralizedEigenSolver prevents usage of custom scalar types.
Status: DECISIONNEEDED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Interoperability (show other bugs)
Version: unspecified
Hardware: All All
: Normal Internal Design
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-26 13:36 UTC by Vadim
Modified: 2018-08-27 14:27 UTC (History)
5 users (show)



Attachments

Description Vadim 2018-08-26 13:36:21 UTC
GeneralizedEigenSolver relies on std::complex implementation for custom scalar types, which is not guaranteed by the standard. In practice, Eigen + boost::multiprecision does not compile under MSVC due to constexpr errors in std::complex. Also, internal implementation of std::complex may not guarantee desired precision requirements with custom scalar types.
Comment 1 Gael Guennebaud 2018-08-26 22:15:19 UTC
I see 4 options:

1 - add an optional template parameter to GeneralizedEigenSolver to let users specify their own custom complex type (defaulted to std::complex)

2 - extend NumTraits to specify which complex type to use for each real type

3 - do nothing and require a specialization of "std::complex<custom_real>"; if there is already a "custom_complex" type, then that task probably boils down to:

namespace std {
class complex<custom_real> : public custom_complex { /* ctors + assignments */ };
}

4 - as suggested many time implement our own complex type, but that solution looks overkill and out of the scope of Eigen.
Comment 2 John Maddock 2018-08-27 11:53:06 UTC
Some background information - this originated as a bug report against Boost.Multiprecision: https://github.com/boostorg/multiprecision/issues/79

As well as our own scalar number types, we also support complex variants of these either via our own wrapper, or via a dedicated backend such as MPC.  Either way the front end is all expression templates.

We also provide a header that provides all the traits classes and "glue code" for our types to work inside Eigen straight out of the box: https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/eigen.hpp

With regard to option 3 above: as users we're really quite limited what we can inject into namespace std:

17.6.4.2.1 Namespace std [namespace.std]
1 The behavior of a C++ program is undefined if it adds declarations or definitions to namespace std or to a namespace within namespace std unless otherwise specified. A program may add a template specialization for any standard library template to namespace std only if the declaration depends on a user-defined type and the specialization meets the standard library requirements for the original template and is not explicitly prohibited.

I'm taking that to mean, that full specializations are OK, but partial ones are not (in practice we can probably get away with one in this case, but in general user-defined partial specialisations can cause ambiguities with std defined ones, which is why the restriction is there).  In addition overloading of std lib non-member functions (complex versions of abs etc) are forbidden, and it's really those non-members that are the main issue for std::complex in practice.  It might be possible to overload these in an associated namespace - I might try that as a temporary workaround later.

Option 1 looks like a bit of a cop-out to me, option 2 looks to be logical option.

There is another related bug here - if the Matrix type is already complex, then you decompose the number type using NumTraits to get the corresponding scalar type, then re-compose it to a std::complex.  

In other words you have one half of the issue solved by already having mappings from complex->scalar in NumTraits and therefore supporting user-defined complex types.  But the reverse mapping scalar->complex is missing.
Comment 3 Christoph Hertzberg 2018-08-27 12:08:32 UTC
I assume there are more cases than GeneralizedEigenSolver which depend on customizing/extending/fixing std::complex, so I don't think option 1 is a good idea.

I was originally favoring option 4, but looking at the ~2000 lines implementation this looks indeed like overkill (especially, when keeping the implementation C++03/11/14/17 compatible ...) -- on the other hand, we might get away with inheriting from std::complex, and just adding "a few" functions. Also, we already have several workarounds for complex which might become obsolete (e.g., missing `EIGEN_DEVICE_FUNC` for `std::complex`.

Regarding option 3, I think you are allowed to do partial specializations, too, as long as they depend on a user-defined type:

    namespace std {
      template<int precision>
      class complex<MyType<precision> > { ... };
    }

But implementing it this way would require re-implementing everything (I assume), i.e., you cannot derive from std::complex anymore (as this would just inherit from itself).

For free functions, you can specialize the corresponding functions in `Eigen::numext::`, if specializing in `std::` is not an option.


But with option 2 this would be easier, as you could have your `complex` specialization in your own namespace, and have all free functions in the same namespace where they can be found by ADL.
Comment 4 Gael Guennebaud 2018-08-27 14:27:14 UTC
ok, option 2 seems to be the easiest then.

Note You need to log in before you can comment on or make changes to this bug.