New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1176 - InnerProduct - too strict assertion on same types
InnerProduct - too strict assertion on same types
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Core - matrix products
3.2
All All
: Normal Unknown
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-03 17:33 UTC by MartinS
Modified: 2016-03-11 17:50 UTC (History)
2 users (show)



Attachments

Description MartinS 2016-03-03 17:33:43 UTC
Eigen seems too strict enforcing its arguments-must-be-of-same-type policy in case of inner products, as demonstrated by the following example

#include <Eigen/Core>

namespace Eigen {
	namespace internal {
		template <> struct scalar_product_traits<float, double> {
			enum { Defined = 1 };
			typedef double ReturnType;
		};
		template <> struct scalar_product_traits<double, float> {
			enum { Defined = 1 };
			typedef double ReturnType;
		};
	}
}

int main(void) {
	Eigen::Matrix<double, 3, 1> dmat;
	Eigen::Matrix<float, 3, 1> fmat;
	
	// [ OK ] 3x1 * 1x3 = 3x3
	dmat * fmat.transpose();
	// [Fail] 1x3 * 3x1 = 1x1
	dmat.transpose() * fmat.transpose();
	
	return 0;
}

As far as I understand, the purpose of the scalar_product_traits template is to allow such mixed-type products via specializations and it indeed works as expected in the outer product case, but not for inner products. 
This seems to me like an oversight that can be easily fixed by removing the STATIC_ASSERTION in GeneralProduct.h in line 208. The GeneralProduct specialization for InnerProducts uses scalar_product_traits anyway, hence the validity of the expression is already ensured.

(Btw. to counter any vectorization arguments against mixed-type arithmetic: Consider 'float' and 'double' in the above example as placeholders for more complex types (like boost::multiprecision or AutoDiffScalar) with appropriately overloaded operators for mixed type arithmetic)
Comment 1 Christoph Hertzberg 2016-03-09 17:05:09 UTC
I guess this was simply not backported to 3.2 -- your code works in the devel branch (if you remove the last .transpose(), which I guess was just a typo), and after this it should also work on 3.2:
https://bitbucket.org/eigen/eigen/commits/72bc00590324

Thanks for the report!
Comment 2 MartinS 2016-03-11 17:50:34 UTC
You're absolutely right, the 2nd .transpose() was unintended.
Thanks for the quick fix!

Now for me to go upstream again, there just needs to be a way to use mixed types within .binaryExpr(...), but I guess I should open another bug report/feature request for that.

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