Enhance plane wave documentation and optimize project plans#7409
Closed
Aunixt wants to merge 48 commits into
Closed
Enhance plane wave documentation and optimize project plans#7409Aunixt wants to merge 48 commits into
Aunixt wants to merge 48 commits into
Conversation
…ork_docs/module_pw_docs
Feat/workflow c
…the real efficiency of new codes on real Linux computer instead of WSL
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds a caching layer with hit/miss statistics for the plane-wave basis (PW_Basis and PW_Basis_K) to avoid redundant recomputation in collect_local_pw and collect_uniqgg. It introduces cache-invalidation hooks on mutating operations, replaces raw owning pointers with std::unique_ptr storage, and adds a copy constructor for PW_Basis. Tests are extended to verify hit/miss behavior.
Changes:
- Add
CacheStats/KCacheStatswith atomic counters and double-checked-locking cachedcollect_local_pw/collect_uniqgg/k-point gcar/gk2 builds. - Replace raw
new[]/delete[]for cached arrays withstd::unique_ptr, and callinvalidate_cache()from parameter/grid mutators and MPI init. - Optimize
setupIndGkby caching selected ig indices in one pass; restructure GPU sync into separatesync_gcar_device_cache/sync_gk2_device_cache.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| source/source_basis/module_pw/pw_basis.h | Declares CacheStats, cache storage, atomics, mutex, copy ctor, invalidate_cache. |
| source/source_basis/module_pw/pw_basis.cpp | Implements copy ctor, cache-storage management, hit/miss tracked collect_local_pw/collect_uniqgg. |
| source/source_basis/module_pw/pw_basis_k.h | Adds KCacheStats, k-cache storage and override of invalidate_cache. |
| source/source_basis/module_pw/pw_basis_k.cpp | Implements caching, sync helpers, optimization of setupIndGk. |
| source/source_basis/module_pw/pw_init.cpp | Inserts invalidate_cache() calls on parameter/grid changes. |
| source/source_basis/module_pw/pw_distributeg.cpp | Invalidates cache when ig2isz/is2fftixy are rebuilt. |
| source/source_basis/module_pw/pw_gatherscatter.h | Removes timer.h include and whitespace cleanup. |
| source/source_basis/module_pw/test/test1-1-1.cpp | Adds asserts for cache pointer stability and hit/miss counts. |
| source/source_basis/module_pw/test_serial/pw_basis_k_test.cpp | Adds asserts for k-cache hit/miss behavior across repeated collect_local_pw calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+19
| PW_Basis::PW_Basis(const PW_Basis& other) | ||
| { | ||
| this->classname = other.classname; |
Comment on lines
1
to
3
| #include "pw_basis.h" | ||
| #include "source_base/global_function.h" | ||
| #include "source_base/timer.h" | ||
| #include <typeinfo> |
Comment on lines
+296
to
301
| const bool gcar_hit = this->gcar_cache_valid.load(); | ||
| const bool gk2_hit = this->gk_cache_valid.load() | ||
| && this->erf_ecut == erf_ecut_in | ||
| && this->erf_height == erf_height_in | ||
| && this->erf_sigma == erf_sigma_in; | ||
| if (this->npwk_max <= 0) |
Comment on lines
+296
to
+300
| const bool gcar_hit = this->gcar_cache_valid.load(); | ||
| const bool gk2_hit = this->gk_cache_valid.load() | ||
| && this->erf_ecut == erf_ecut_in | ||
| && this->erf_height == erf_height_in | ||
| && this->erf_sigma == erf_sigma_in; |
Comment on lines
+59
to
+65
| struct KCacheStats : public PW_Basis::CacheStats | ||
| { | ||
| std::uint64_t gcar_hits = 0; | ||
| std::uint64_t gcar_misses = 0; | ||
| std::uint64_t gk2_hits = 0; | ||
| std::uint64_t gk2_misses = 0; | ||
| }; |
Comment on lines
143
to
+146
| this->nz = ibox[2]; | ||
| this->nxy =this->nx * this->ny; | ||
| this->nxyz = this->nxy * this->nz; | ||
| this->invalidate_cache(); |
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.
This pull request introduces a comprehensive caching mechanism for the plane-wave basis (
PW_Basis) and its k-point variant (PW_Basis_K). The changes aim to improve memory management, thread safety, and performance by reducing redundant allocations and computations, and by tracking cache usage statistics. The most important changes are grouped below.Caching and Memory Management
gg,gdirect,gcar,ig2igg,gg_uniqinPW_Basis;gcar,gk2inPW_Basis_K), replacing manualnew/deletewithstd::unique_ptrfor safer and more efficient memory management. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7L31-R94),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R241-R263),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R320-R354),[[4]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R385-R388),[[5]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7L264-R422),[[6]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36L24-R25),[[7]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R50-R87),[[8]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36L252-R350))clear_owned_cache()andclear_k_cache_storage()methods to centralize cache cleanup and reset pointers, ensuring proper resource deallocation. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R104-R150),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R50-R87))Thread Safety and Performance
std::mutex cache_mutex) and double-checked locking in cache access to ensure thread safety and avoid redundant cache population in multi-threaded scenarios. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R241-R263),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R320-R354),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36L252-R350))[[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-dffedd9da498a900ee8970451b3831da1e2b31d9fc00a5d315b70e4fe450d49cR155-R181),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R104-R150),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R50-R87))Cache Usage Statistics
get_cache_stats()andreset_cache_stats()methods in bothPW_BasisandPW_Basis_Kto expose cache usage statistics (hits, misses, and memory usage), aiding in performance analysis and debugging. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-dffedd9da498a900ee8970451b3831da1e2b31d9fc00a5d315b70e4fe450d49cR64-R75),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-dffedd9da498a900ee8970451b3831da1e2b31d9fc00a5d315b70e4fe450d49cR155-R181),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R50-R87))Algorithmic and Code Quality Improvements
collect_uniqgg()to usestd::vectorinstead of manual heap allocation, improving safety and clarity. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R320-R354),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R385-R388),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7L264-R422))ModuleBase::timer::start/end) to key methods for improved profiling. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R241-R263),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R320-R354),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R172-R188),[[4]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R214),[[5]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R240),[[6]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36L252-R350))Interface and Consistency Changes
PW_Basisto ensure proper copying of all relevant members, including cache state. ([[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R17-R72),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-dffedd9da498a900ee8970451b3831da1e2b31d9fc00a5d315b70e4fe450d49cR64-R75))[[1]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-7ac753c93ead4c20908852ca73646f1218f0a876b69f29b5f1d075792affc0d7R449-R454),[[2]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R143),[[3]](https://github.com/deepmodeling/abacus-develop/pull/7409/files#diff-556a9660ae30c7f1504a796d73c177d6dc17716b4dc79f7d356d94fc549e9f36R172-R188))These changes collectively make the plane-wave basis classes more robust, thread-safe, and performant, while also providing better introspection into cache behavior for developers.### Reminder
Linked Issue
Fix #...
Unit Tests and/or Case Tests for my changes
What's changed?
Any changes of core modules? (ignore if not applicable)