Conversation
|
Example output for a single (cold-cache)
|
Codecov Report❌ Patch coverage is
... and 48 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
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. |
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>
|
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
left a comment
There was a problem hiding this comment.
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.
* 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>
Instruments the index manipulation, contraction and factorization kernels with
@timeit_debugsections, compiled away by default and enabled viaTensorKit.enable_timers!().Section labels carry a category prefix (
symmetry:/bookkeeping:/alloc:/dense:), andTensorKit.timer_summary()aggregates exclusive times into per-category totals that sum to the total measured time. The@cachedmacro 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