Perf: buffer shared text CSR output and avoid AtomPair copies - #7931
Merged
Conversation
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.
Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.Linked Issue
No linked issue: this is a small, independently scoped optimization of the shared text CSR writer, identified while profiling an ABACUS 3.11-based H0 exporter. No exporter-specific code or numerical changes are included.
What's changed?
std::endlcalls inSparseMatrix::printToCSRwith newline characters. Preserve the final flush, so a completed payload is still visible when the function returns and flush failures still set stream state / throw when enabled.AtomPairthroughconst auto&inOutput_HContainer::write_single_R. The loop only reads the selected R block; copying the pair also copies its R metadata and potentially all owned matrix blocks.The intended user-visible difference is lower text-output overhead. Bytes, ordering, thresholds, precision, headers, and append conventions are unchanged. Intermediate line-by-line visibility becomes buffered; the final per-payload flush is retained. This does not change stream options such as
unitbuf.Scope: HContainer-backed text H/S, DM, matrix derivatives, asynchronous overlap, Hamiltonian terms, and some DeePKS outputs share these writers. Independent legacy map-based sparse writers and binary serializers are not optimized by this PR. No full-SCF speedup is claimed.
The 3.11 writer was moved to
source_base/module_outupstream; this PR uses the current develop layout.Unit Tests and/or Case Tests for my changes
Built with GCC 10.5, CMake 3.26.4, oneMKL 2024.2, Intel MPI 2021.13, and existing GTest 1.11.0; Release,
BUILD_TESTING=ON,ENABLE_MPI=ON, optional Libxc/ELPA/LibRI disabled.OMP_NUM_THREADS=1,MKL_NUM_THREADS=1.Commands run (
test_builddenotes the configured MPI build, 8 build CPUs):Results: all 6 CTest entries passed. 41 distinct GoogleTests passed (the two-rank binary gather case skips in the one-process entry and passes on both ranks in its dedicated MPI entry). Existing compatibility tests cover CSR structure headers/readback/append, DM/spin output, legacy sparse and derivative output, and binary real/complex/order/append/MPI behavior. New tests verify exactly one explicit sync per CSR payload.
Initial local setup attempts failed before tests: compute-node CMake was absent, the MPI-disabled build did not register these targets, and the prebuilt GTest needed its matching libstdc++. These were resolved using existing shared tools/libraries; no project build configuration or dependency was changed.
Not run: full SCF, GPU, or DeePKS end-to-end workflows, and a whole-program performance benchmark. Their numerical/operator paths are untouched; DeePKS applicability was inspected at its shared-writer call sites. The dedicated two-rank integration test exercises binary H/S gathering, not text gathering.
Governance Notes