Reduce CI strain by splitting test files even more - #517
Conversation
|
I'd be somewhat surprised if this helps too much, it might be worth it to measure where the actual compilation is happening but I'd expect most of it to be in the tensor kernels, which are largely shared between finite and infinite implementations. A different thing could be to just split up the tests into more groups, i.e. just use more runners, for example splitting off time evolution algorithms from groundstate/leading boundary algorithms etc |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Yeah, testing both fast and normal mode, it seems that there's no global speedup this way. I'm looking into |
|
Good point, I almost forgot about that PR. I'll try and take that back up, since I do think that should help out quite severely. |
|
I haven't tested yet with TensorKit precompilation, but at least within MPSKit the compilation bottlenecks are (unsurprisingly) |
|
So I have 3 data points on running 2 of the compile-heaviest tests (MPOHamiltonian and changebonds) with and without TensorKit precompilation. I did this on the MPSKit main branch, so whatever (minimal or no) effects this branch currently has plays no role. The run-to-run variance is massive, but TensorKit precompilation does help. This being a statistical coincidence is (1/2)^6 so I think it's safe to say that it's beneficial to precompile. It is fairly modest on my machine though. It fluctuates between 3-17% speedup, on average ~9% which is about 4 minutes per file. Given that, I believe that the best thing to do is to set up precompilation within MPSKit as well. Afterwards we can see if test splitting can fill in remaining gaps. |
|
I'm not entirely sure what you measured of course (did you include the tensorkit precompilation time?), but there is the additional effect that sometimes the CI can actually cache the TensorKit precompilation entirely, in which case this is more or less a free speedup. |
|
No, indeed my first run did have an additional cost in precompiling TK (15 minutes on my machine, though I think it was under strain from other tasks), so with the small amount of tests I did it's more or less break-even. But it's clear when running all the tests multiple times, its effects are more and more prominent. I ran |
…o bd/ci-strain
borisdevos
changed the title
|
The documentation error is fixed in #508 and can be ignored here. |
…o bd/ci-strain
Hoists the repeated (planar, U1, SU2) sectortype triple and MPS-state (D, d, elt) list into shared TestSetup constants, following the existing SCHEDULERS precedent. Keeps the full 3-way sweep only in hamiltonian/infinite.jl; operators_misc/lazysum.jl (the largest generic compile-time driver in the suite) drops SU2 since LazySum's arithmetic/ dispatch bookkeeping doesn't depend on which sector type is plugged in, and SU2 fusion-tree correctness stays covered elsewhere. Also pairs the eltype x space sweep in hamiltonian/finite.jl with zip instead of a nested loop, since the two axes don't interact, halving its combinations. Also fixes a changelog typo and removes a dead unasserted variable in hamiltonian/infinite.jl. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Drops the confusing "_misc" naming (three unrelated folders were all called some variant of "misc"): algorithms_misc/ -> algorithms/, operators_misc/ -> operators/, misc/ -> internals/. Pulls the sector/symmetry-convention tests (sector_conventions.jl, multifusion.jl) out of their separate "misc" homes into a new symmetries/ folder, since both are about the same thing. old_bugs.jl's four regression testsets are moved into the topic folder each bug actually belongs to (groundstate/, algorithms/changebonds.jl, states/finitemps.jl) instead of a shared grab-bag file, following the "Regression: ..." comment convention already used in changebonds.jl. Also fixes CONTRIBUTING.md's test-suite description and example commands, which were already stale relative to the prior reorg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses review feedback: prefer computing the reduced/full spacelist directly in an if/else rather than building the full list and then truncating it in a separate fast_tests && (...) statement. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@borisdevos apologies for hijacking your PR, I reshuffled some stuff around, but overall definitely like the idea. If everything passes now I'd be happy to merge this. |
|
No worries, I've been a bit busy recently to work on PRs so I appreciate your work :) |
2 participants
Description
I first considered splitting symmetric and non-symmetric code, but then many files would be super tiny. The next best thing I thought of was the finite-infinite split. This is in no way definitive, just wanted to get the ball rolling on considering this, especially before merging #509 which is ridiculously expensive.
Summary:
None of the content or hyperparameters of tests have been changed, it's really just copy-paste, introducing some lists and truncating them depending on
fast_tests, or skipping certain test sets based on this.The downside to this approach is that all checks will take longer to finish, but at least they'll finish instead of time out. And who knows, maybe the time saved on compiling might be enough to be comparable with full parallelisation.
Checklist
julia --project=test test/runtests.jl, or the relevant subset)docs/src/)[Unreleased]indocs/src/changelog.md, if this PR is user-facing (new feature, behavior change, bug fix, deprecation, or removal)Edit: Ignore everything above except the fast flag, we split the algorithms and operators directories into more directories based on operator types, since the main bottleneck is eigensolvers or linear solvers specialised to some operator type. Some files remain split based on finite/infinite, purely for readability. No content has been changed for the tests. The fast flag still remains.