NWChemEx / CoupledCluster

Other
3 stars 5 forks source link

DRAFT: Bug fix and performance improvement for SYCL CCSD_T #4

Closed lfmeadow closed 7 months ago

lfmeadow commented 9 months ago

Status

Brief Description

  1. unrolling loops in methods/cc/ccsd_t/ccsd_t_all_fused_nontcCuda_Hip_Sycl.cpp makes a significant performance improvement on NVIDIA because it allows clang to put the array elements in registers.
  2. Recently discovered missing synchronization allows main thread to continue before host tasks are complete. Shows up on AMD.

Detailed Description (optional)

TODOs and/or Questions (optional)

CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

abagusetty commented 7 months ago

@lfmeadow Thanks for the PR. Could you please resolve the conflicts, such that we can close this PR

lfmeadow commented 7 months ago

Ok i will to do that today.

Sent from my iPhone

On Nov 20, 2023, at 8:55 AM, Abhishek Bagusetty @.***> wrote:



@lfmeadowhttps://github.com/lfmeadow Thanks for the PR. Can you push your changes to main branch instead and resolve the conflicts for these two files.

— Reply to this email directly, view it on GitHubhttps://github.com/NWChemEx-Project/CoupledCluster/pull/4#issuecomment-1819453437, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABH5YK4AKUFZH57T4IOBQB3YFODN5AVCNFSM6AAAAAA4VJKYR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJZGQ2TGNBTG4. You are receiving this because you were mentioned.Message ID: @.***>

ajaypanyala commented 7 months ago

@abagusetty @lfmeadow It was a minor conflict, so I just fixed it.

lfmeadow commented 7 months ago

Cool thanks Im not with Codeplay anymore btw. Working for AMD

Sent from my iPhone

On Nov 20, 2023, at 9:03 AM, Ajay Panyala @.***> wrote:



@abagusettyhttps://github.com/abagusetty @lfmeadowhttps://github.com/lfmeadow It was a minor conflict, so I just fixed it.

— Reply to this email directly, view it on GitHubhttps://github.com/NWChemEx-Project/CoupledCluster/pull/4#issuecomment-1819467391, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABH5YK6V76KTFJ6ZHQZYAZLYFOENZAVCNFSM6AAAAAA4VJKYR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJZGQ3DOMZZGE. You are receiving this because you were mentioned.Message ID: @.***>

abagusetty commented 7 months ago

Cool thanks Im not with Codeplay anymore btw. Working for AMD Sent from my iPhone On Nov 20, 2023, at 9:03 AM, Ajay Panyala @.> wrote:  @abagusettyhttps://github.com/abagusetty @lfmeadowhttps://github.com/lfmeadow It was a minor conflict, so I just fixed it. — Reply to this email directly, view it on GitHub<#4 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABH5YK6V76KTFJ6ZHQZYAZLYFOENZAVCNFSM6AAAAAA4VJKYR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJZGQ3DOMZZGE. You are receiving this because you were mentioned.Message ID: @.>

Congrats @lfmeadow!

@ajaypanyala please go ahead with the merge