[Solver.tpp] add warning message to updateHessianMatrix and updateLinearConstraintsMatrix for RowMajor

[Naoki-Hiraoka]

Hello. I am a user of OsqpEigen.

OsqpEigen::Solver::updateHessianMatrix and OsqpEigen::Solver::updateLinearConstraintsMatrix do not support RowMajor matrix
because OsqpEigen::SparseMatrixHelper::eigenSparseMatrixToTriplets does not support RowMajor matrix.

I have added warning message to inform users of this.

[Naoki-Hiraoka]

Supplementary explanation.

The order of the Triplets returned by OsqpEigen::SparseMatrixHelper::eigenSparseMatrixToTriplets is different depending on whether input matrix is RowMajor or ColMajor. OsqpEigen::Solver::evaluateNewValues ​​can not compare correctly in RowMajor case.

[S-Dafarra]

Hi @Naoki-Hiraoka,

thanks for the PR, and apologies for the late answer.

OsqpEigen::SparseMatrixHelper::eigenSparseMatrixToTriplets does not support RowMajor matrix.

To be honest, this is not really clear to me.
The function OsqpEigen::SparseMatrixHelper::eigenSparseMatrixToTriplets iterates over the non-zero elements with the following for loop

osqp-eigen/include/OsqpEigen/SparseMatrixHelper.tpp

Lines 139 to 144 in bba25ca

	for (int k=0; k < eigenSparseMatrix.outerSize(); ++k){
	for (typename Eigen::SparseCompressedBase<Derived>::InnerIterator it(eigenSparseMatrix,k); it; ++it){
	tripletList[nonZero] = Eigen::Triplet<T>(it.row(), it.col(), static_cast<T>(it.value()));
	nonZero++;
	}
	}

Depending on if the matrix is RowMajor or ColMajor the outer size and the inner size change, but both should be supported.

In particular,

The order of the Triplets returned by OsqpEigen::SparseMatrixHelper::eigenSparseMatrixToTriplets is different depending on whether input matrix is RowMajor or ColMajor. OsqpEigen::Solver::evaluateNewValues ​​can not compare correctly in RowMajor case.

is indeed expected. This problem arises when the solver is initialized with a ColMajor matrix and then updated with a RowMajor matrix. In this case, evaluateNewValues should return false and reinitialize the problem.

So, in the end RowMajor is supported.

[Naoki-Hiraoka]

Thank you for response!

I'm sorry for my inaccurate description, but I think RowMajor is not supported.

As you say, OsqpEigen::SparseMatrixHelper::eigenSparseMatrixToTriplets and OsqpEigen::Solver::evaluateNewValues support RowMajor matrix. (My previous comments were incorrect.)

This problem arises when the solver is initialized with a ColMajor matrix and then updated with a RowMajor matrix. In this case, evaluateNewValues should return false and reinitialize the problem.

However, the solver is always initialized with a ColMajor matrix
because OsqpEigen::SparseMatrixHelper::createOsqpSparseMatrix converts the input matrix to ColMajor.
And OsqpEigen::SparseMatrixHelper::osqpSparseMatrixToTriplets does not support RowMajor matrix.
And the order of the Triplets returned by OsqpEigen::SparseMatrixHelper::osqpSparseMatrixToTriplets is always ColMajor.

osqp-eigen/include/OsqpEigen/SparseMatrixHelper.tpp

Line 13 in bba25ca

Eigen::SparseMatrix<typename Derived::value_type, Eigen::ColMajor> colMajorCopy; //Copying into a new sparse matrix to be sure to use a CSC matrix

[S-Dafarra]

However, the solver is always initialized with a ColMajor matrix
because OsqpEigen::SparseMatrixHelper::createOsqpSparseMatrix converts the input matrix to ColMajor.

Ah I see, good catch! Indeed, the core problem is that an osqp sparse matrix is always ColMajor. That's why we convert a RowMajor matrix when initializing the solver. But then when updating evaluateNewValues would always return false.

I guess that the problem then is to improve evaluateNewValues such that the check is not dependent on the triplets' order, or to modify eigenSparseMatrixToTriplets such that the output is the same independently of ColMajor or RowMajor.

[traversaro]

I guess that the problem then is to improve evaluateNewValues such that the check is not dependent on the triplets' order, or to modify eigenSparseMatrixToTriplets such that the output is the same independently of ColMajor or RowMajor.

Hello @S-Dafarra and @Naoki-Hiraoka, some time has passed, do you know if this problem is still present? If it is, probably it could make sense to move this PR to an issue? Thanks!

[S-Dafarra]

I guess that the problem then is to improve evaluateNewValues such that the check is not dependent on the triplets' order, or to modify eigenSparseMatrixToTriplets such that the output is the same independently of ColMajor or RowMajor.

Hello @S-Dafarra and @Naoki-Hiraoka, some time has passed, do you know if this problem is still present? If it is, probably it could make sense to move this PR to an issue? Thanks!

Hi @traversaro, I think that the problem is still there. Corresponding issue: #168

[traversaro]

Great, thanks! I think at this point we can close this PR, and open a new one if someone wants to solve #168 .