Refactor: new gint module #5869
Open
Conversation
This file contains 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
This reverts commit 414446a.
mohanchen
added
Features Needed
mohanchen
reviewed
Jan 19, 2025
mohanchen
reviewed
Jan 22, 2025
| void Veff<OperatorLCAO<TK, TR>>::contributeHR() | ||
| if(this->nspin == 2) | ||
| { | ||
| this->current_spin = 1 - this->current_spin; |
There was a problem hiding this comment.
this code is a bit strange, may need to rewrite
| std::vector<double> ddphi_zz; | ||
| ModuleBase::matrix* fvl_thread = nullptr; | ||
| ModuleBase::matrix* svl_thread = nullptr; | ||
| if(isforce_) |
There was a problem hiding this comment.
I feel it is a better design to let 'isforce_' and 'isstress_' parameters to be input parameters of this function 'cal_fvl_svl_()', and enforce them to be 'const'
| dphi_x.resize(phi_len); | ||
| dphi_y.resize(phi_len); | ||
| dphi_z.resize(phi_len); | ||
| dphi_x_DM.resize(phi_len); |
There was a problem hiding this comment.
I think it would be better to use 'dm' instead of 'DM' in the new grid integral code.
| phi_op.set_phi_dphi(nullptr, dphi_x.data(), dphi_y.data(), dphi_z.data()); | ||
| for (int is = 0; is < nspin_; is++) | ||
| { | ||
| phi_op.phi_mul_dm(dphi_x.data(), *DMRGint_vec_[is], true, dphi_x_DM.data()); |
There was a problem hiding this comment.
it would be better to use 'dmr_gint' instead of 'DMRGint'
| //======================== | ||
| double dr3_; | ||
|
|
||
| std::shared_ptr<HContainer<double>> hRGint_; |
There was a problem hiding this comment.
maybe use 'hr_gint_'instead of 'hRGint_'?
mohanchen
reviewed
Jan 22, 2025
| public: | ||
| // constructor | ||
| UnitCellInfo( | ||
| Vec3d unitcell_vec1, |
There was a problem hiding this comment.
change to 'const Vec3d &unitcell_vec1'?
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.
Background
The original
module_gintwas disorganized, with input and output for different grid integration tasks mixed together. Additionally, the overall framework for grid integration was poorly structured. This PR rewrites the grid integration framework. The new code is located undersource/module_hamilt_lcao/module_gint/new_gint.Since the new code is not yet complete—GPU version grid integration, for example, has not been implemented in this PR—it is considered an experimental PR. Only a portion of the old grid integration interfaces have been replaced with the new grid integration interfaces and the old grid integration code will be retained until the new grid integration code is fully completed, at which point it will be removed. Moreover, the new grid integration code will only be compiled when
-DNEW_GINTis set during CMake configuration.Unit Tests and/or Case Tests for my changes
What's changed?
module_gintcode, if-DNEW_GINTis set during CMake configuration, some interfaces of old version ofmodule_gintwill be replaced with new version.Performance Comparison
Based on current testing, the new grid integration code shows slightly higher efficiency due to more detailed optimizations。 Below are the results from running the si64 in tests/performance on my computer using
OMP_NUM_THREADS=4 mpirun -n 4 abacus: