Skip to content

Conversation

@guitargeek
Copy link
Contributor

This PR reverts the following two commits:

I merged these PRs believing that their addressing a real problem for ATLAS users, as muhammadalhroob is an ATLAS collaboration member.

However, I learned later in another PR by the same contributor that the point of the changes to TMatrix was just to try out AI. That made me have second thoughts about the TMatrix optimizations, and I would suggest to revert them at this point for several reasons:

  • TMatrix never claimed to have good performance, and users that need performant matrix multiplications use other libraries. So optimizing TMatrix with AI risks to break things without a clear benefit. If users are sticking with TMatrix, they seemingly don't mind slower matrix multiplications and appreciate instead the simplicity and stability of TMatrix.
  • The new code introduced different code branches for different matrix sizes where we can't be sure if they are covered by the tests.
  • We don't want to encourage AI-driven development for the fun of it if there is no motivation for the change.

I know that at least @silverweed is agreeing with the revert.

FYI, @muhammadalhroob. I hope you can understand the reasoning, and that this is nothing personal. We are ready to welcome and review your contributions to ROOT anytime! It was my totally mistake that I didn't ask about the motivation in your first PR.

@github-actions
Copy link

Test Results

    22 files      22 suites   3d 16h 54m 10s ⏱️
 3 791 tests  3 791 ✅ 0 💤 0 ❌
80 280 runs  80 280 ✅ 0 💤 0 ❌

Results for commit d7f7327.

@dpiparo dpiparo self-requested a review December 24, 2025 09:24
@guitargeek guitargeek merged commit 8eb4e58 into root-project:master Dec 24, 2025
32 of 33 checks passed
@guitargeek guitargeek deleted the revert_tmatrix branch December 24, 2025 12:44
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