Skip to content

[math][GenVector] Add access operator for Rotation3D #18480

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mdessole
Copy link
Contributor

This Pull request:

Fixes https://root-forum.cern.ch/t/tvector3-rotate-to-arbitrary-rotation-using-xyzvector/63244

Alternative to #18462

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@mdessole mdessole self-assigned this Apr 23, 2025
@mdessole mdessole marked this pull request as ready for review April 23, 2025 11:56
@mdessole mdessole requested a review from lmoneta as a code owner April 23, 2025 11:56
Copy link

github-actions bot commented Apr 23, 2025

Test Results

    17 files      17 suites   3d 15h 40m 4s ⏱️
 2 730 tests  2 730 ✅ 0 💤 0 ❌
45 035 runs  45 035 ✅ 0 💤 0 ❌

Results for commit 7d0a363.

♻️ This comment has been updated with latest results.

@ferdymercury
Copy link
Contributor

Thanks a lot Monica for this alternative suggestion! If we decide to go this way, could you also implement the helper function:

/** copy-doxygen-docu from other pull request, too*/
         template <class Vector>
         Vector Rotate(const Vector & v, double alpha, const Vector & axis) {
            Vector a = v;
            ROOT::Math::VectorUtil::Rotate(a, ROOT::Math::Rotation3D(ROOT::Math::AxisAngle(axis, alpha)));
            return a;
         }

as well as 'rescue' the TRotation alpha fmod fix, and the fixed typoes in the documentation from the linked PR, that could be then closed ?

@mdessole mdessole requested a review from guitargeek April 24, 2025 09:05
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