LTO support

[Krzmbrzl]

No description provided.

[Copilot]

Pull request overview

Adds project-wide optimization/LTO enablement via CMake and wires it into libraries, utilities, tests, and benchmarks, with accompanying CI and dependency updates.

Changes:

• Introduces target_set_optimization_flags() and applies it broadly to build targets (libraries, utilities, tests, benchmarks).
• Updates benchmarks to use a minimal MBPT operator registry and adds a “quick benchmarks” mode used in CI.
• Refreshes several pinned external dependency tags and bumps GitHub Actions versions.

Reviewed changes

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

Show a summary per file

File	Description
utilities/external-interface/CMakeLists.txt	Enables optimization/LTO flags for the external-interface utility target.
utilities/cost_analysis/CMakeLists.txt	Enables optimization/LTO flags for the cost_analysis utility target.
utilities/CMakeLists.txt	Enables optimization/LTO flags for all utilities built via the loop.
tests/unit/test_optimize.cpp	Adjusts lambdas and loop variable binding (contains a compile-breaking lambda change).
tests/unit/test_cache_manager.cpp	Explicitly ignores return values from store() to silence unused-result warnings.
tests/unit/CMakeLists.txt	Applies warning + optimization flags to unit test object libraries and the final unit test binary.
tests/integration/CMakeLists.txt	Applies optimization/LTO flags to integration test executables.
SeQuant/domain/mbpt/op.cpp	Avoids unused-variable warnings in assertion-only loops via [[maybe_unused]].
SeQuant/core/eval/eval.hpp	Initializes ResultPtr members explicitly to {}.
external/versions.cmake	Updates tracked tags/versions for several external dependencies.
CMakeLists.txt	Applies optimization/LTO flags to non-interface SeQuant module targets and SeQuant-bliss.
cmake/compiler.cmake	Adds target_set_optimization_flags() (contains LTO detection/enablement logic issues).
benchmarks/wick.cpp	Adds configurable input cap and uses make_minimal_registry() in benchmark context.
benchmarks/coupled_cluster.cpp	Adds configurable max rank and sets a minimal default MBPT context for the benchmark.
benchmarks/CMakeLists.txt	Applies warning/optimization flags and adds SEQUANT_QUICK_BENCHMARKS compile defs in Debug/quick mode.
.github/workflows/formatting_check.yml	Bumps checkout action major version.
.github/workflows/docs.yml	Bumps checkout/setup-python action major versions.
.github/workflows/cmake.yml	Bumps actions versions; enables quick benchmarks and adds a benchmark dry-run step.
.github/workflows/benchmark_compare.yml	Bumps actions/github-script, checkout, cache, and upload-artifact major versions.

Suppressed comments (2)

tests/unit/test_optimize.cpp:772

• This lambda no longer captures batch but still references it; since batch is a local variable this is a compile error.

        auto batch_fn = [](Index const&) -> std::size_t { return batch; };

tests/unit/test_optimize.cpp:814

• This lambda references the local variable batch without capturing it, which will fail to compile.

        auto batch_fn = [](Index const&) -> std::size_t { return batch; };

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ajay-mk]

@Krzmbrzl Could you update the CMake options table in docs?

[evaleev]

Code review: LTO support

Reviewed the full diff (CI action bumps, the new target_set_optimization_flags, benchmark OpRegistry fixes + "quick" mode, dependency bumps, warning fixes). 8 findings inline, ordered by severity below; the LTO flag-detection claims were verified empirically on Apple Clang 17 with a standalone CMake reproduction.

#	Where	Severity
1	cmake/compiler.cmake:95	medium/high
2	missed target_set_optimization_flags consumers	medium
3	cmake/compiler.cmake:67 (+ benchmarks/CMakeLists.txt:23)	medium
4	benchmarks/coupled_cluster.cpp:28 (maxRank)	medium
5	benchmarks/wick.cpp:131 (nMbptInputs)	low/medium
6	cmake/compiler.cmake:122	low
7	cmake/compiler.cmake:81,84-86	low
8	benchmarks/coupled_cluster.cpp:18	low

Verified as correct — no action needed

• SeQuant/core/eval/eval.hpp:560 — ResultPtr left/right = {} is safe: Frame remains an aggregate and all three construction sites (581, 656, 695) use designated initializers.
• tests/unit/test_optimize.cpp:730,772,814 — dropping the batch capture compiles; all three are std::size_t const batch = 1;, so the read is a constant expression and not an odr-use.
• --benchmark_dry_run=true exists in the tracked Google Benchmark (src/benchmark.cc:764 in v1.9.4, and SEQUANT_OLDEST_GOOGLEBENCHMARK_VERSION is 1.9.3), and build/benchmarks/sequant_benchmarks is the correct output path (no CMAKE_RUNTIME_OUTPUT_DIRECTORY is set).
• check_cxx_compiler_flag("-flto;-ffat-lto-objects" ...) does pass both flags — Internal/CheckCompilerFlag.cmake expands the list unquoted into CMAKE_REQUIRED_DEFINITIONS.
• make_minimal_registry() covers everything the fixed benchmarks need (t, h, f, g, plus L/R backing t::l/t::r), and spintrace genuinely needs no registry — so benchmarks/spintrace.cpp correctly went untouched.
• SEQUANT_QUICK_BENCHMARKS being option()-declared inside benchmarks/ is fine with the CI's unconditional -D...=ON; it only produces an "unused variable" notice in the valgrind/sanitize jobs where benchmarks are off.

Not verified

The external/versions.cmake tag bumps (utfcpp v4.1.1, CLI11 v2.7.2, spdlog v1.17.0, Catch2 v3.15.3, googlebenchmark v1.9.5, pybind11 v3.1.0) could not be checked — this review ran without network access, so git ls-remote against the upstream repos was unavailable. Someone should confirm those tags exist and that the SEQUANT_OLDEST_* floors are still accurate. Same for the actions/*@v7 / @v6 / @v9 bumps in the workflows.

[evaleev]

[3 — medium] The CMAKE_BUILD_TYPE STREQUAL "Debug" guard is a no-op under multi-config generators.

With Ninja Multi-Config, Xcode, or Visual Studio, CMAKE_BUILD_TYPE is empty, so this early return never fires and -flto=auto is applied unconditionally to every config, including Debug — slowing Debug builds substantially and degrading debuggability.

The identical guard at benchmarks/CMakeLists.txt:23 has the same problem in reverse: under a multi-config generator, SEQUANT_BENCH_MAX_CC_RANK / SEQUANT_BENCH_MAX_WICK_INPUTS are never defined for the Debug config, so the "don't let Debug benchmarks take forever" protection silently disappears.

Fix: use a $<CONFIG:Debug> generator expression for the flags (e.g. wrap the options in $<$<NOT:$<CONFIG:Debug>>:...>) rather than testing CMAKE_BUILD_TYPE at configure time.

[Krzmbrzl]

Correct though I don't think the added complexity is really worth the hassle. This can be dealt with once someone actually uses Windows to do SeQuant developing.