Analytical PCM 2nd derivative #302
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
Contributor
henryw7
commented
Jan 10, 2025
•
henryw7
commented
- Change all finite difference implementation of PCM second derivative to analytical form.
- Bug fix in C-PCM gradient. The absolute error is at 1e-9 level for a gradient calculation, but will be magnified to 1e-4 level when computing hessian using finite difference.
- Bug fix in SS(V)PE gradient. SS(V)PE and IEF-PCM do not share gradient algorithm. We were misled by QChem 6.1 implementation.
henryw7
changed the title
wxj6000
reviewed
Jan 24, 2025
| ngrids = grids.shape[0] | ||
| # n_charge_sum_per_thread = 1 # means every thread processes one pair and one grid | ||
| # n_charge_sum_per_thread = ngrids # or larger number gaurantees one thread processes one pair and all grid points | ||
| n_charge_sum_per_thread = 10 |
There was a problem hiding this comment.
Why is n_charge_sum_per_thread hard-coded as 10?
| ngrids = grids.shape[0] | ||
| # n_charge_sum_per_thread = 1 # means every thread processes one pair and one grid | ||
| # n_charge_sum_per_thread = ngrids # or larger number gaurantees one thread processes one pair and all grid points | ||
| n_charge_sum_per_thread = 10 |
There was a problem hiding this comment.
Why is it hardcoded as 10?
| vK_1_d2K_q += cupy.einsum('Adi,i,BDi->ABdD', vK_1_dot_dDdx, A, dSdx_dot_q) | ||
| vK_1_d2K_q += cupy.einsum('Adi,BDi->ABdD', vK_1D_dot_dAdx, dSdx_dot_q) | ||
| vK_1_d2K_q += cupy.einsum('Adi,BDi->BADd', vK_1_dot_dDdx, dAdx_dot_Sq) | ||
| vK_1_d2K_q += cupy.einsum('Adi,i,BDi->BADd', vK_1_dot_dDdx, A, dSdx_dot_q) |
There was a problem hiding this comment.
split the 3-tensor contraction into 2 2-tensor contractions. Or simply make use of broaddcasting, A * dSdx_dot_q
| matrix_d2S[i*n + j + n2 * 7] = 0.0; | ||
| matrix_d2S[i*n + j + n2 * 8] = 0.0; | ||
| } else { | ||
| matrix_d2S[i*n + j ] = dx * dx * S_direct_product_prefactor + S_xyz_diagonal_prefactor; |
There was a problem hiding this comment.
Set S_direct_product_prefactor = 0 if i == j? The if-else logic may slow down GPU memory access.
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.