Split vnl_pw.cpp into several smaller files - #7869
Merged
Merged
Conversation
added 10 commits
August 28, 2026 07:10
- Move getvnl<FPTYPE, Device> template body + 4 explicit instantiations (CPU/GPU float/double) from vnl_pw.cpp to new file vnl_pw_getvnl.cpp (~165 lines) - Update CMakeLists.txt and Makefile.Objects accordingly - vnl_pw.cpp reduced from 1797 to 1641 lines Verification: cd build_max_para_test && make -j 30 => build passed, no undefined reference
- One variable per line for better readability and maintainability - Rename pointer variables: _tab -> tab_ptr, _indv -> indv_ptr, _nhtol -> nhtol_ptr, _nhtolm -> nhtolm_ptr (avoid leading underscore, use _ptr suffix) Verification: cd build_max_para_test && make -j 30 => build passed
Rule deepmodeling#8 in Required Baseline: declare one variable per line; do not use comma-separated declarations. This improves readability, makes diffs cleaner, and helps debugging with single-step execution.
- Replace int* h_atom_nh/na/nb with std::vector<int> - Replace ModuleBase::Vector3<double>* _gk with std::vector<Vector3> - Use .data() to get raw pointer for GPU sync operations - Remove all delete[] calls (RAII handles cleanup automatically) Benefits: - Exception safety (no memory leak if exception thrown) - Cleaner code (no manual memory management) - Follows modern C++ best practices Verification: cd build_max_para_test && make -j 30 => build passed
- Add file-level @file/@brief documentation - Add getvnl() function documentation with workflow steps - Add comment explaining explicit template instantiations
- Move compute_qrad(), radial_fft_q() (CPU matrix version), radial_fft_q<FPTYPE,Device>() (template version), and 4 explicit template instantiations to vnl_pw_qrad.cpp - Replace new[]/delete[] with std::vector in compute_qrad() - Add doxygen comments for all functions - Update CMakeLists.txt and Makefile.Objects Verification: cd build_max_para_test && make -j 30 => build passed
- Move cal_effective_D(), newq(), newd_so(), newd_nc() to vnl_pw_deeq.cpp - Replace new[]/delete[] qnorm with std::vector in newq() - Add doxygen comments for all functions - Update CMakeLists.txt and Makefile.Objects - Fix blas_connector.h include path (source_base/module_external/) Verification: cd build_max_para_test && make -j 30 => build passed
Step 4 of vnl_pw.cpp split: - Move Cal_C(), CG(), init_vnl_alpha() into new vnl_pw_alpha.cpp - Wrap whole file with #ifdef __LCAO - Drop dead commented-out getvnl_alpha block - Replace new[]/delete[] with std::vector for jl/aux in init_vnl_alpha - Sync CMakeLists.txt and Makefile.Objects Verified: make -j 30 in build_max_para_test passes.
Step 5 of vnl_pw.cpp split: - Move init_vnl() into new vnl_pw_init_vnl.cpp - Replace new[]/delete[] with std::vector for jl/aux inside the tab fill loop - Sync CMakeLists.txt and Makefile.Objects Verified: make -j 30 in build_max_para_test passes.
Step 6 of vnl_pw.cpp split (final): - Keep only ctor/dtor, release_memory(), init(), print_vnl(), rescale_vnl() and the get_*_data<T>() template specializations - Drop now-unused includes (clebsch_gordan, math_integral, math_polyint, math_sphbes, math_ylmreal, parallel_reduce, vnl_op, parallel_comm) - Add file-level doxygen comment describing the new module layout Verified: make -j 30 in build_max_para_test passes.
mohanchen
added
Refactor
Critsium-xy
approved these changes
Aug 28, 2026
Critsium-xy
left a comment
Critsium-xy
left a comment
Collaborator
There was a problem hiding this comment.
OK, but this file should be deleted
The empty CONNECT file was unintentionally added in commit 3b53f58 during the vnl_pw_qrad split. It is not referenced by any code, build file, or script, so it is safe to delete.
…/vnl pseudopotential code - convert pseudopot_cell_vl local buffers (vloc1d/aux1/aux) and member zp to std::vector - convert pseudopot_cell_vnl::indv_ijkb0 to std::vector and drop manual delete - use std::fill/std::copy instead of ZEROS/COPYARRAY helpers on vectors
…vloc - print_vloc takes out_element_info and global_out_dir explicitly instead of reading PARAM internally - callers invoke print_vloc right after init_vloc at all four call sites
…get_energy Cache PARAM.inp.nspin into a local const to reduce repeated global parameter accesses, and add the missing newline at end of file.
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
2 participants
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.
Split vnl_pw.cpp into several smaller files and refactor some files, replace new/delete with std::vector