Skip to content

Replace tranposes with .mT#558

Merged
jank324 merged 11 commits intomasterfrom
faster-transpose
Sep 24, 2025
Merged

Replace tranposes with .mT#558
jank324 merged 11 commits intomasterfrom
faster-transpose

Conversation

@jank324
Copy link
Copy Markdown
Member

@jank324 jank324 commented Sep 10, 2025

Description

Sub- / replacement PR for #538.

We've seen that the fastest transpose operation in PyTorch seems to be x.mT.

Screenshot 2025-09-19 at 15 42 15

Motivation and Context

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)

Checklist

  • I have updated the changelog accordingly (required).
  • My change requires a change to the documentation.
  • I have updated the tests accordingly (required for a bug fix or a new feature).
  • I have updated the documentation accordingly.
  • I have reformatted the code and checked that formatting passes (required).
  • I have have fixed all issues found by flake8 (required).
  • I have ensured that all pytest tests pass (required).
  • I have run pytest on a machine with a CUDA GPU and made sure all tests pass (required).
  • I have checked that the documentation builds (required).

Note: We are using a maximum length of 88 characters per line.

@jank324
Copy link
Copy Markdown
Member Author

jank324 commented Sep 10, 2025

benchmark_commits_plot

@jank324
Copy link
Copy Markdown
Member Author

jank324 commented Sep 19, 2025

benchmark_commits_plot benchmark_commits_plot

@jank324
Copy link
Copy Markdown
Member Author

jank324 commented Sep 19, 2025

I think in this case the code readability is even slightly improved. Considering the speed up is only 200 ns per call, and the number of calls not that large, the lack of a visible overall speedup is not surprising. Despite this, I think we can and should merge this.

@jank324 jank324 requested review from Hespe, Copilot and cr-xu September 19, 2025 14:02
@jank324 jank324 marked this pull request as ready for review September 19, 2025 14:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves performance by replacing various PyTorch transpose operations with the faster .mT property. Based on benchmarking, .mT is the fastest transpose operation in PyTorch for matrix transposition.

  • Replace torch.transpose() and .transpose() calls with .mT across multiple files
  • Update code formatting to use single-line expressions where applicable
  • Add performance documentation to contribution guidelines

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_statistics.py Replace .T with .mT in test covariance calculation
cheetah/utils/statistics.py Use .mT for transpose in weighted covariance matrix computation
cheetah/utils/kde.py Replace .transpose() with .mT in kernel density estimation
cheetah/track_methods.py Update transpose operations in transfer matrix calculations
cheetah/particles/particle_beam.py Replace transpose operations in particle transformation
cheetah/accelerator/screen.py Update transpose usage in beam reading methods
cheetah/accelerator/element.py Use .mT in first-order transfer map tracking
cheetah/accelerator/dipole.py Replace transpose in dipole transfer map calculations
cheetah/accelerator/cavity.py Update transpose operations in cavity tracking
CONTRIBUTING.md Add performance guidance for transpose operations
CHANGELOG.md Document performance improvements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread CONTRIBUTING.md Outdated
Comment thread cheetah/accelerator/dipole.py Outdated
Comment thread cheetah/track_methods.py Outdated
jank324 and others added 2 commits September 24, 2025 12:11
Co-authored-by: Christian Hespe <christian.hespe@desy.de>
@jank324
Copy link
Copy Markdown
Member Author

jank324 commented Sep 24, 2025

benchmark_commits_plot benchmark_commits_plot benchmark_commits_plot

@jank324
Copy link
Copy Markdown
Member Author

jank324 commented Sep 24, 2025

These results are really weird ... especially because the drift-kick-drift code wasn't touched. In theory this must therefore be noise. 🤔

@jank324 jank324 requested review from Hespe and Copilot September 24, 2025 10:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread cheetah/track_methods.py
Comment thread cheetah/accelerator/dipole.py
@jank324 jank324 merged commit a8e2637 into master Sep 24, 2025
10 checks passed
@jank324 jank324 deleted the faster-transpose branch September 24, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants