[#97] Modernize Eigen MRPd support and attitude conversions - #1464
Open
schaubh wants to merge 11 commits into
Open
[#97] Modernize Eigen MRPd support and attitude conversions#1464schaubh wants to merge 11 commits into
schaubh wants to merge 11 commits into
Conversation
schaubh
added
bug
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abdf08ddd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| TEST(eigenMRP, testCoefficientConstructorsAndAccessors) { | ||
| const double rawCoefficients[3] = {0.10, -0.20, 0.30}; // [-] | ||
|
|
||
| Eigen::MRPd fromScalars(0.10, -0.20, 0.30); |
There was a problem hiding this comment.
AGENTS.md requires trailing inline unit comments for numeric literal assignments/definitions when the quantity has physical meaning. These added MRP coefficient definitions are dimensionless attitude quantities, but this line (and the similar new MRP/vector coefficient definitions in this test) lacks a // [-] unit comment, so it violates the repository's review rule.
Useful? React with 👍 / 👎.
schaubh
force-pushed
the
feature/97-eigen-MRPd
branch
from
July 6, 2026 13:55
731e11b to
d5b6f41
Compare
Update MRP conversion helpers to accept and return Eigen::MRPd directly, and convert clear attitude read sites away from generic Vector3d handling. Add focused test coverage updates and a release-note snippet.
Update hub and state-effector dynamics methods to pass attitude MRPs as Eigen::MRPd instead of generic Vector3d values. Convert spacecraft and spacecraftSystem callers to construct MRPd values at hub-state reads, and clean up now-redundant attitude casts in affected state effectors.
Fix Eigen MRP conversion and operator behavior so DCM/AngleAxis conversions, composition, vector rotation, scalar casts, and setFromTwoVectors behave consistently with Basilisk MRP conventions. Keep DCM-to-MRP conversion on the short-rotation set and clarify MRP norm/normalization documentation. Expand avsEigenMRP unit coverage across constructors, maps, assignment, composition, shadow sets, Bmat, inverse/conjugate, angular distance, and casts. Update the MuJoCo integration attitude comparison to be robust to quaternion sign while preserving strict checks on the remaining qpos states.
Migrate remaining internal semantic MRP attitudes from Eigen::Vector3d detours to Eigen::MRPd where this does not cross message, state-manager, Python/SWIG, or estimator-state storage boundaries. Keep raw coefficient vectors at those boundaries and convert with MRPd only where attitude operations are performed.
schaubh
force-pushed
the
feature/97-eigen-MRPd
branch
from
July 9, 2026 13:30
d5b6f41 to
864ac0e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR modernizes Eigen MRP support and moves selected attitude handling to
Eigen::MRPdwhere the values semantically represent MRPs. The commits are organized as small library upgrades and then downstream usage updates, ending with a standalone release-note snippet.Key changes include more robust
Eigen::MRPdconversions, short-rotation quaternion/DCM handling, scalar casts for mapped MRP expressions, clearer assignment diagnostics, and expanded MRP unit test coverage. Dynamics attitude interfaces now pass MRP attitudes asEigen::MRPdwhile preserving raw vector storage at state/message/SWIG boundaries.Verification
Validated with:
make -C dist3 test_avsEigenMRPmake -C dist3 test_avsEigenSupportdist3/architecture/utilities/tests/test_avsEigenMRPdist3/architecture/utilities/tests/test_avsEigenSupport.venv/bin/pytest -q src/simulation/mujocoDynamics/_GeneralModuleFiles/_UnitTest/test_integration.pygit diff --check origin/develop...HEADThe MRP GoogleTest suite was expanded to cover constructors, maps, casts, assignment paths, DCM/AngleAxis conversion branch behavior, composition, shadow sets,
setFromTwoVectors(), coefficient arithmetic, and small-vector handling.Documentation
Updated
avsEigenMRP.hDoxygen comments to clarify MRP norm/normalization behavior, coefficient arithmetic, supported assignment paths, and helper internals. Added a release-note snippet documenting theEigen::MRPdconversion updates and theeigenMRPd2CArray()signature change. UpdatedAGENTS.mdreview guidance to check forEigen::MRPduse where appropriate.Future work
None