Skip to content

Conversation

@wxj6000
Copy link
Contributor

@wxj6000 wxj6000 commented Mar 27, 2025

The issue (see #75)

Current int3c2e kernels allocate a large amount of local memory, which may lead to out of memory when cupy memory pool occupies 90% of global memory. This PR tries to resolve this issue.

TODOs

  • Reduce local memory in int3c2e gradient and hessian
    • Reduce 2x gsize -> 1x gsize in gradient
    • Reduce 4x gsize -> 1x gsize in Hessian
  • Map each task to one block for high angular momentum

Additionally

In DF/grad/JK kernels, the intermediates (3,nao) are eliminated. The derivatives w.r.t atomic positions are directly calculated in CUDA kernels.

@wxj6000 wxj6000 requested a review from Copilot April 4, 2025 23:23
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 41 out of 45 changed files in this pull request and generated 1 comment.

Files not reviewed (4)
  • gpu4pyscf/lib/gint/CMakeLists.txt: Language not supported
  • gpu4pyscf/lib/gint/bpcache.cu: Language not supported
  • gpu4pyscf/lib/gint/g2e.cu: Language not supported
  • gpu4pyscf/lib/gint/g2e.h: Language not supported
Comments suppressed due to low confidence (1)

gpu4pyscf/df/grad/uhf.py:174

  • [nitpick] The new variable names 'ej' and 'ek' are not self-explanatory. Consider using more descriptive names (e.g., 'grad_j' and 'grad_k') to clearly indicate their purpose.
ej = -ej

Comment on lines 66 to 67
exit()

Copy link

Copilot AI Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of 'exit()' here forces an early termination of the example, which may be unintended. Consider removing it to allow the script to complete its execution.

Suggested change
exit()

Copilot uses AI. Check for mistakes.
@wxj6000 wxj6000 marked this pull request as ready for review April 4, 2025 23:55
@wxj6000 wxj6000 requested a review from Copilot April 5, 2025 16:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 39 out of 44 changed files in this pull request and generated no comments.

Files not reviewed (5)
  • gpu4pyscf/lib/gint/CMakeLists.txt: Language not supported
  • gpu4pyscf/lib/gint/bpcache.cu: Language not supported
  • gpu4pyscf/lib/gint/g2e.cu: Language not supported
  • gpu4pyscf/lib/gint/g2e.h: Language not supported
  • gpu4pyscf/lib/gint/g3c2e.cu: Language not supported
Comments suppressed due to low confidence (2)

gpu4pyscf/df/int3c2e.py:446

  • [nitpick] The variable naming has been updated from 'vj' to 'ej' (and similarly for 'vk' to 'ek'). Please update the associated inline comments to reflect this change for clarity.
            ej = cupy.zeros([natm,3], order='C')

gpu4pyscf/df/grad/uhf.py:174

  • [nitpick] The inline comment about variables remaining in Cartesian coordinates appears to reference the old 'vj/vk' names. Consider updating or removing the comment to avoid confusion given the new 'ej/ek' naming.
        ej = -ej

@sunqm
Copy link
Collaborator

sunqm commented Apr 7, 2025

Please check the size of the output binary file. If the int3c2e code leads to a very big sized wheel, adjust the unrolling level.

@wxj6000 wxj6000 merged commit 3017839 into master Apr 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants