Repository navigation
perf(LabelTensor): 1.6x faster extract (67.0 ms -> 41.6 ms per round) - #847
Merged
Merged
Conversation
dario-coscia
approved these changes
Nov 1, 2026
dario-coscia
left a comment
Member
There was a problem hiding this comment.
Great job! Good to go for me, thanks for your contribution
Member
|
@dexhunter Not blocking but can you open an issue and reference it in the PR? Please see the CONTRIBUTING.md file. I'll change the merge to dev instead of master branch, so please do a rebase on dev so we can merge it. |
dario-coscia
requested review from
a team,
annaivagnes and
ndem0
as code owners
November 1, 2026 17:09
GiovanniCanali
requested changes
Nov 1, 2026
GiovanniCanali
left a comment
Collaborator
There was a problem hiding this comment.
Great enhancement! Thank you for your contribution @dexhunter.
Could you please rebase your branch to clean up the commit history and ensure that only the commits relevant to this PR are included?
Other than that, everything looks good and is ready to merge!
LabelTensor.extract looked up a single label with list.index twice and rebuilt the result with LabelTensor(tensor, labels), which re-checked the labels of every dimension. Find each label position once, check only the dimensions that changed (same checks and messages as the constructor), and attach the labels to the result directly. Extract benchmark: 67.0 ms -> 41.6 ms per round (37.9% less time). Values, labels, view/copy behaviour and error messages are unchanged. Refs PINA-org#846 Co-Authored-By: Aiden <aiden@weco.ai>
dexhunter
force-pushed
the
perf/label-tensor-extract
branch
from
November 1, 2026 20:34
38fe701 to
8862fb8
Compare
Author
|
@dario-coscia @GiovanniCanali Thank you for the review.
|
GiovanniCanali
approved these changes
Nov 2, 2026
GiovanniCanali
pushed a commit
that referenced
this pull request
Nov 5, 2026
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
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.
TL;DR
LabelTensor.extractfaster. Operators, problems and many tests call it.test_extract_3Dcall shapes and related ones): 67.0 ms -> 41.6 ms, 37.9% less time.pytest testsgives 4135 passed, the same as onmaster.Fixes #846
What & Why
Before,
extractdid extra work on each call. For a single label, it calledlist.indextwo times. At the end,LabelTensor(tensor, labels)checked the labels of every dimension again, also the dimensions that did not change.Now the method finds each label position once. It checks only the dimensions it changed, with the same checks and messages as the constructor. Then it attaches the labels to the result directly. The indexing code stays the same.
The output stays identical. Values, shapes, labels, view-or-copy behaviour and error messages are the same as before.
Changes
pina/_src/core/label_tensor.py: onlyLabelTensor.extractchanges.LabelTensor.__new__and set_labels, so the full label check does not run again.Results
Time per call, CPU, one thread, best of 7 runs, mean of two runs:
For large copies, the tensor copy takes most of the time, so the change does not help. These cases do not get slower.
Why this is safe and easy to maintain
The change is small and stays inside one method. The indexing code does not change. Only the label checks at the end run for fewer dimensions. The behaviour and output are unchanged, also for error messages.
tests/test_label_tensor,tests/test_conditionandtests/test_operator.pycover theextractcall shapes, and the full suite passes.Validation
timeitscript with the cases above, run onmasterand on this branch.pytest testsgives 4135 passed on this branch and onmaster.pytest tests/test_label_tensor tests/test_condition tests/test_operator.pygives 255 passed.black --check pinapasses.python -m timeit -s "import torch; from pina import LabelTensor; t = LabelTensor(torch.rand(20, 5), list('abcde'))" "t.extract(['a', 'c', 'e'])"Checklist
AI assistance
Codex (gpt-6.1-sol) generated the code change as candidates, and Weco Observe tracked them. Claude Code chose the candidate, ran the benchmarks and tests, and wrote this description. Automated checks compared the output with
master. No human review happened before this PR was opened.Weco share link: https://dashboard.weco.ai/share/J2bBmekDU6T7ou_GmDOfm22xOofUKgW4