Adding WeldJoint and ConstantSampler #146
Conversation
There was a problem hiding this comment.
The general procedure of implementing the statespace::dart::WeldJoint state space and helper functions (createDifferentiableFor, createTestableFor, createProjectableFor) looks reasonable.
However, I don't understand why ConstantSampler is necessary. Isn't that equivalent to a RnBoxConstraint with equal upper and lower bounds?
|
@mkoval I initially had that, which works fine, but it would unnecessarily try to "sample" when it can simply return a fixed value, so I thought this would be cleaner? I can switch back to using |
There was a problem hiding this comment.
If we were starting from scratch, I'd suggest using RnBoxConstraint to avoid having to implement - and test 😬 - a new constraint class. Since you've already implemented ConstantSampler, I see no harm in keeping it. 😃
I am happy to merge this as it stands if you add Rn to the name of the new constraint class and write unit tests for the new functionality. 👍
| /// ConstantSampler for RealVectorStates. | ||
| /// Stub sampler for WeldJoint or any fixed constant state space. | ||
| class ConstantSampler | ||
| : public constraint::Sampleable |
There was a problem hiding this comment.
Rename ConstantSampler to RnConstantSampler since this only works on an Rn state space.
|
|
||
| //============================================================================= | ||
| class ConstantSamplerSampleGenerator | ||
| : public constraint::SampleGenerator |
There was a problem hiding this comment.
- Rename to
RnConstantSamplerSampleGenerator(see above). - Define this class in an anonymous
namespacesince it is only used in this file.
src/statespace/dart/WeldJoint.cpp
Outdated
| void WeldJoint::convertPositionsToState( | ||
| const Eigen::VectorXd& _positions, StateSpace::State* _state) const | ||
| { | ||
| setValue(static_cast<State*>(_state), _positions); |
There was a problem hiding this comment.
I believe we don't need to convert any value here because the dimension of WeldJoint is zero.
src/statespace/dart/WeldJoint.cpp
Outdated
| void WeldJoint::convertStateToPositions( | ||
| const StateSpace::State* _state, Eigen::VectorXd& _positions) const | ||
| { | ||
| _positions = getValue(static_cast<const State*>(_state)); |
There was a problem hiding this comment.
Nothing to get values from _state, which is zero dimensional. It would be sufficient to resize _positions to zero as _positions.resize(0); (see above)
|
Addressed @mkoval 's comments. |
|
@jslee02 Sorry to ask you, but would you mind writing unit tests for the two classes? Thanks a lot!! |
|
Changes Unknown when pulling de46056 on statespace/WeldJoint into ** on master**. |
|
Changes Unknown when pulling 51446a6 on statespace/WeldJoint into ** on master**. |
|
Changes Unknown when pulling 6a3080c on statespace/WeldJoint into ** on master**. |
|
Changes Unknown when pulling 6a3080c on statespace/WeldJoint into ** on master**. |
This PR adds the following:
and uses ConstantSampler as a sampler for WeldJoint.
This change is