tapkee icon indicating copy to clipboard operation
tapkee copied to clipboard

Aliasing error during eigen decomposition

Open crud89 opened this issue 6 years ago • 3 comments

Hey there,

I found your library really useful so far and I am really surprised by its efficiency. However, I am experiencing some aliasing issues. The following code fails during embedding. Note that I have defined TAPKEE_CUSTOM_INTERNAL_NUMTYPE as float. 1

// Create a matrix with 512x512 32-dim features.
tapkee::DenseMatrix descriptors(32, 262144);

// Randomly fill the matrix.
for (int i(0); i < 32; ++i)
for (int j(0); j < 262144; ++j)
	descriptors(i, j) = static_cast<float>(std::rand()) / static_cast<float>(RAND_MAX);

// Reduce to dim 8 using PCA.
tapkee::ParametersSet parameters = tapkee::kwargs[
	tapkee::method = tapkee::PCA,
	tapkee::target_dimension = 8
];

// Perform PCA
tapkee::TapkeeOutput result = tapkee::initialize()
	.withParameters(parameters)
	.embedUsing(descriptors);

Basically I want to reduce a set of 512x512 32 dimensional features to 8D, which I randomly initialize for demonstration purposes. When ran in debug-mode (under Win64), Eigen detects aliasing:

aliasing detected during transposition, use transposeInPlace() or evaluate the rhs into a temporary using .eval()

The error is raised during eigen decomposition (eigendecomposition.hpp):

//! Eigen library dense implementation of eigendecomposition-based embedding
template <class MatrixType, class MatrixOperationType>
EigendecompositionResult eigendecomposition_impl_dense(const MatrixType& wm, IndexType target_dimension, unsigned int skip)
{
	timed_context context("Eigen library dense eigendecomposition");

	DenseSymmetricMatrix dense_wm = wm;
	dense_wm += dense_wm.transpose();	// Invalid.
	dense_wm /= 2.0;
	DenseSelfAdjointEigenSolver solver(dense_wm);
	
	// ...
}

It relates to the issues described here. Changing this line to dense_wm += dense_wm.transpose().eval(); resolves the issue. I think transposeInPlace() should also work, however I was not able to get it to compile that way.

Did I miss something or is this an actual bug?!

1 Also note that the documentation says TAPKEE_CUSTOM_NUMTYPE, which is actually incorrect.

crud89 avatar Feb 01 '19 09:02 crud89

Hey, thanks for the report.

It seems to be an actual bug. The eval thing resolves that indeed but it would bump the memory usage 2x for a moment. Let me check if there is a memory-efficient solution.

lisitsyn avatar Feb 01 '19 10:02 lisitsyn

It seems that eval should be here.

I just added it in ec3514033c036795ca819d3ac68b61a58d24ac53

lisitsyn avatar Feb 01 '19 11:02 lisitsyn

Thank you for the fast fix!

crud89 avatar Feb 01 '19 11:02 crud89