Skip to content

Add TimerOutputs-based timers for internal kernels - #525

Merged
lkdvos merged 3 commits into
mainfrom
ld-timer
Sep 8, 2026
Merged

lkdvos merged 3 commits into
mainfrom
ld-timer

Conversation

@lkdvos

@lkdvos lkdvos commented Sep 2, 2026

Copy link
Copy Markdown
Member

Instruments the index manipulation, contraction and factorization kernels with @timeit_debug sections, compiled away by default and enabled via TensorKit.enable_timers!().

Section labels carry a category prefix (symmetry: / bookkeeping: / alloc: / dense:), and TensorKit.timer_summary() aggregates exclusive times into per-category totals that sum to the total measured time. The @cached macro times lookups separately from miss-path construction. While timing, get_num_*_threads() return 1 so that the (single-task) timer is never touched concurrently; the guard const-folds away when disabled, leaving production paths unchanged (verified via benchmark against main).

Adds TimerOutputs (1.x, for recursive enable_debug_timings) as a dependency, a "Profiling and timers" manual page, and tests.

🤖 Generated with Claude Code

@lkdvos lkdvos linked an issue Sep 2, 2026 that may be closed by this pull request
@lkdvos

lkdvos commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Example output for a single (cold-cache) permute + svd_compact on an SU2Space(0 => 4, 1//2 => 4, 1 => 2) rank-4 tensor:

TensorKit.timer_summary()

    symmetry:  385μs ( 20.6%)   548KiB  39 sections
 bookkeeping:  195μs ( 10.5%)   159KiB  137 sections
       alloc: 25.4μs (  1.4%)   134KiB  3 sections
       dense: 1.19ms ( 63.8%)   228KiB  58 sections
       other: 70.7μs (  3.8%)  25.7KiB  3 sections
TensorKit.print_timers() — full call tree
 Section                                                  ncalls    time    %tot     avg
───────────────────────────────────────────────────────────────────────────────────────────
 svd_compact!                                                  1  1.20ms   64.2%  1.20ms
 ├─ dense: lapack                                              5  1.11ms   59.7%   223μs
 ├─ bookkeeping: cache sectorstructure                        24  19.8μs    1.1%   825ns
 │  └─ bookkeeping: compute sectorstructure                    1  2.15μs    0.1%  2.15μs
 └─ bookkeeping: cache degeneracystructure                    15  7.87μs    0.4%   525ns
 permute!                                                      1   579μs   31.0%   579μs
 └─ braid!                                                     1   579μs   31.0%   579μs
    ├─ bookkeeping: cache treebraider                          1   488μs   26.1%   488μs
    │  └─ symmetry: compute treebraider                        1   481μs   25.8%   481μs
    │     ├─ symmetry: recoupling matrices                     1   420μs   22.5%   420μs
    │     │  ├─ bookkeeping: cache fsbraid                    37   371μs   19.9%  10.0μs
    │     │  │  └─ symmetry: compute fsbraid                  37   347μs   18.6%  9.37μs
    │     │  └─ bookkeeping: repack                           37  16.0μs    0.9%   433ns
    │     ├─ bookkeeping: fusionblocks                         1  37.7μs    2.0%  37.7μs
    │     ├─ bookkeeping: cache sectorstructure                2  7.66μs    0.4%  3.83μs
    │     ├─ bookkeeping: cache degeneracystructure            2  6.41μs    0.3%  3.21μs
    │     └─ bookkeeping: sort                                 1  4.50μs    0.2%  4.50μs
    ├─ dense: tensoradd                                       29  36.9μs    2.0%  1.27μs
    ├─ dense: pack                                             8  21.4μs    1.1%  2.67μs
    ├─ dense: unpack                                           8  12.2μs    0.7%  1.53μs
    ├─ dense: recouple mul!                                    8  6.00μs    0.3%   750ns
    └─ alloc: buffers                                          1   735ns    0.0%   735ns
 alloc: initialize_output                                      1  50.6μs    2.7%  50.6μs
 └─ bookkeeping: cache degeneracystructure                     2  33.6μs    1.8%  16.8μs
 bookkeeping: cache degeneracystructure                        1  29.6μs    1.6%  29.6μs
 alloc: copy_input                                             1  9.30μs    0.5%  9.30μs

(some deeply nested sectorstructure/degeneracystructure sub-branches elided)

@lkdvos
lkdvos requested a review from leburgel September 2, 2026 09:36
@codecov

codecov Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.92857% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/planar/planaroperations.jl 96.29% 1 Missing ⚠️
src/tensors/indexmanipulations.jl 97.43% 1 Missing ⚠️
src/tensors/treetransformers.jl 92.30% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/TensorKit.jl 17.24% <100.00%> (+3.44%) ⬆️
src/auxiliary/caches.jl 90.10% <100.00%> (+0.57%) ⬆️
src/auxiliary/timers.jl 100.00% <100.00%> (ø)
src/factorizations/factorizations.jl 84.61% <ø> (+15.38%) ⬆️
src/factorizations/matrixalgebrakit.jl 93.83% <100.00%> (+9.15%) ⬆️
src/tensors/linalg.jl 82.93% <100.00%> (+15.35%) ⬆️
src/tensors/tensoroperations.jl 96.53% <100.00%> (+5.95%) ⬆️
src/planar/planaroperations.jl 73.18% <96.29%> (+1.86%) ⬆️
src/tensors/indexmanipulations.jl 89.18% <97.43%> (+12.48%) ⬆️
src/tensors/treetransformers.jl 94.84% <92.30%> (-3.01%) ⬇️

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leburgel leburgel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is pretty amazing.

Comment thread src/auxiliary/timers.jl Outdated
Comment thread src/factorizations/matrixalgebrakit.jl Outdated
Comment thread src/factorizations/matrixalgebrakit.jl Outdated
Comment thread src/factorizations/matrixalgebrakit.jl Outdated
Comment thread src/planar/planaroperations.jl Outdated
Comment thread src/planar/planaroperations.jl
Comment thread src/tensors/indexmanipulations.jl Outdated
Comment thread src/tensors/indexmanipulations.jl Outdated
Comment thread src/tensors/indexmanipulations.jl Outdated
Comment thread src/tensors/indexmanipulations.jl Outdated
Comment thread src/tensors/indexmanipulations.jl
Comment thread src/tensors/indexmanipulations.jl
Comment thread src/tensors/tensoroperations.jl Outdated
Comment thread src/tensors/tensoroperations.jl
Comment thread src/tensors/tensoroperations.jl
Comment thread src/auxiliary/timers.jl Outdated
Comment thread src/auxiliary/timers.jl
@Jutho

Jutho commented Sep 3, 2026

Copy link
Copy Markdown
Member

Great work. There are quite a few places where I can see it is not entirely straightforward where to put the timer exactly. In particular, for the calls to dense linear algebra blocks/subblocks, do we want one "dense" timer entry around the whole loop over the (sub)blocks (thereby also potentially including other work aside from the dense call) or inside the loop just around the actual dense call.

Anyway, I left some comments, feel free to ignore them.

lkdvos and others added 2 commits September 8, 2026 13:10
Instrument the index manipulation, tensor contraction and factorization
kernels with `@timeit_debug` sections that are compiled away by default
and can be enabled with `TensorKit.enable_timers!()`. Section labels
carry a category prefix (`symmetry:` / `bookkeeping:` / `alloc:` /
`dense:`), and `TensorKit.timer_summary()` aggregates the exclusive time
of each section into per-category totals, to measure the split between
fusion tree manipulations, block structure bookkeeping, allocations and
the actual dense tensor kernels.

The `@cached` macro additionally times cache lookups separately from
miss-path construction for all memoized functions. While timers are
enabled, `taskforeach` regions run serially since a `TimerOutput` may
only be manipulated from a single task; the guard const-folds away when
timers are disabled, leaving the production paths unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- drop the redundant `permute!` section and label `braid!` as `permute!/braid!`
- time the trivial-sector fast paths in `braid!`/`transpose!` as `dense: tensoradd`
- rename `dense: lapack` to `dense: MatrixAlgebraKit`
- move the `dense: trace` section inside the loop in `planartrace!` so the
  untimed `planar_trace` coefficients are not attributed to `dense`
- move `_cached_category` next to the `@cached` macro; clarify the
  `timeit_debug_enabled` comment and `timers_enabled` docstring

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lkdvos

lkdvos commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Ok my robot friend accidentally also replied to your comments, please ignore that...

I've integrated most of your suggested changes, in general tried to be reasonable with the granularity of the sections, but I think for detailed information a profiler is probably still the right tool, and this should serve mostly as a generic indication of rough percentages, which is also why there are a number of cases where I think the overhead of the timer would dominate the signal so I left them out deliberately.

@Jutho Jutho left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this looks very useful and the timer structuring decisions all seem very reasonable to me, and can be finetuned further at some later point if need be.

@lkdvos
lkdvos enabled auto-merge (squash) September 8, 2026 21:41
@lkdvos
lkdvos merged commit d95e413 into main Sep 8, 2026
85 of 86 checks passed
@lkdvos
lkdvos deleted the ld-timer branch September 8, 2026 23:10
@lkdvos lkdvos mentioned this pull request Sep 20, 2026
lkdvos referenced this pull request Sep 21, 2026
* Draft changelog for v0.17.2

Consolidates the Unreleased section (which already included the real
entries added by #526/#532 on merge) with entries for the remaining
PRs merged since v0.17.1 (#487-#535), and retitles it as 0.17.2.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Bump version to v0.17.2

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix confirmed small bugs from the pre-release audit

- isunitspace: require dim(V) == 1 for GenericUnit sectors (#537)
- GradedSpace ⊕/supremum: check unit homogeneity of the result (#538)
- isconj(::ComplexSpace): return isdual(V) instead of always true (#539)
- multi_associator: return a vector, not a scalar, on early-exit for
  GenericFusion (#540)
- split(f, 0): use leftunit(f.coupled) instead of indexing an empty
  uncoupled tuple (#541)
- repartition: return a Pair in the identity branch, matching every
  other branch (#542)
- Mooncake scalar_pullback: accumulate into the tangent instead of
  overwriting it (#543)
- rand/randn/randexp/randisometry(rng, T, space): fix one(domain) typo (#544)
- pinv(::DiagonalTensorMap): fix inverted atol/rtol defaulting and
  empty-tensor throw (#545)
- t1 / t2: promote to a float scalartype, matching t1 \ t2 (#546)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Add changelog entry for the audit bugfixes

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address fable review findings on the audit bugfixes

- split(f, 0): also guard innerlines_extended construction, which
  still indexed the empty uncoupled tuple for a 0-leg tree
- pinv(::DiagonalTensorMap): use eps (not sqrt(eps)) for the default
  rtol, matching _default_rtol's convention and dense LinearAlgebra.pinv

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address tuicr review comments on the audit bugfixes

- pinv(::DiagonalTensorMap): reuse _default_rtol instead of
  duplicating its formula
- Add regression tests for split(f, 0) on a genuine 0-leg tree and
  for multi_associator's early-exit branch on a GenericFusion sector

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix planar issues after MPSKit test rerun

* harden Mooncake scalar pullback

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.

Support TimerOutputs.jl instrumentation

3 participants