Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation for both ColMajor/RowMajor + provide a define. #89

Merged
merged 3 commits into from
Apr 12, 2015

Conversation

bchretien
Copy link
Member

Users can use the CMake option "STORAGE_ORDER" to change the storage order used by RobOptim. It defaults to ColMajor (Eigen's default), and RowMajor can be used for hypothetical increased performance in some scenarios (e.g. when mostly using impl_gradient to fill the
Jacobian matrices).

Details

In RobOptim, gradient_t describes a row of the Jacobian matrix, but until now, it was defined as a column vector. A new rowVector_t is now available (vector_t is still a column vector), and in the dense case, gradient_t is just a rowVector_t. The sparse case is a bit different, since vector_t/rowVector_t are dense vectors but gradient_t is a sparse vector (now a RowMajor sparse vector for consistency with the dense traits).

Also, I encountered the following Eigen bug: http://eigen.tuxfamily.org/bz/show_bug.cgi?id=980
This will be fixed in the next Eigen release.

Users can use the CMake option "STORAGE_ORDER" to change the storage
order used by RobOptim. It defaults to ColMajor (Eigen's default),
and RowMajor can be used for hypothetical increased performance in
some scenarios (e.g. when mostly using impl_gradient to fill the
Jacobian matrices).
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 92.41% when pulling 5597f65 on bchretien:master into 039e21f on roboptim:master.

@bchretien
Copy link
Member Author

Note: plugins need to be tested before merging this.

@bchretien
Copy link
Member Author

Things seem to be working as expected. The Ipopt plugin and the Python bindings needed a quick fix.

Benjamin Chrétien added 2 commits April 8, 2015 11:22
gradient_t was previously used for this, but gradient_t describes
row vectors while derivative_t describes column vectors.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 92.41% when pulling a4dcac7 on bchretien:master into 039e21f on roboptim:master.

@bchretien
Copy link
Member Author

I added an extra type, derivative_t, which is merely a typedef to a column vector. It should be used instead of gradient_t (row vector) for NTimesDerivableFunction (i.e. in roboptim-trajectory).

bchretien pushed a commit to bchretien/roboptim-trajectory that referenced this pull request Apr 8, 2015
Also comply with the change to gradient_t (row vector).
Cf. roboptim/roboptim-core#89
bchretien pushed a commit to bchretien/roboptim-trajectory that referenced this pull request Apr 8, 2015
Also comply with the change to gradient_t (row vector).
Cf. roboptim/roboptim-core#89
bchretien pushed a commit to bchretien/roboptim-trajectory that referenced this pull request Apr 8, 2015
Also comply with the change to gradient_t (row vector).
Cf. roboptim/roboptim-core#89
bchretien pushed a commit to bchretien/roboptim-trajectory that referenced this pull request Apr 8, 2015
Also comply with the change to gradient_t (row vector).
Cf. roboptim/roboptim-core#89
bchretien referenced this pull request in fdarricau/roboptim-core-manifold Apr 8, 2015
bchretien added a commit that referenced this pull request Apr 12, 2015
Fix compilation for both ColMajor/RowMajor + provide a define.
@bchretien bchretien merged commit 2be0719 into roboptim:master Apr 12, 2015
bchretien pushed a commit to roboptim/roboptim-trajectory that referenced this pull request Apr 12, 2015
Also comply with the change to gradient_t (row vector).
Cf. roboptim/roboptim-core#89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants