search

cctbx / cctbx_project

Computational Crystallography Toolbox
https://cci.lbl.gov/docs/cctbx
Other
207 stars 111 forks source link

Streamline the two exascale simulation kernels. #954

Closed nksauter closed 5 months ago

nksauter commented 5 months ago

Debranch kernel now a) uses vec3 rather than Holton-vec4; b) marshals all mos-tics and phi-tics into one list of crystal orientations; c) aligns exact order of operations (*,/) with kokkosSpotsKernel.

nksauter commented 5 months ago

debranch_maskall_kernel is now about 45% faster than before on the cry11ba simulation test. Simulated images are identical (before vs after) when doing a spot check to integer precision on each pixel.

mewall commented 5 months ago

Does the diffuse simulation need to be checked or can we safely assume it will be the same?

Get Outlook for iOShttps://aka.ms/o0ukef

From: nksauter @.> Sent: Thursday, January 11, 2024 4:19:17 PM To: cctbx/cctbx_project @.> Cc: Wall, Michael E @.>; Review requested @.> Subject: [EXTERNAL] Re: [cctbx/cctbx_project] Streamline the two exascale simulation kernels. (PR #954)

@nksauterhttps://urldefense.com/v3/__https://github.com/nksauter__;!!Bt8fGhp8LhKGRg!C40sgSpkkK1ddG182hnELPxXJOEYg3lz6rHSuS241wEnP5tizyLRKZxDJbFFs28YcdPkqYjaGJ92yA9B9bYYdFw9$ requested your review on: #954https://urldefense.com/v3/__https://github.com/cctbx/cctbx_project/pull/954__;!!Bt8fGhp8LhKGRg!C40sgSpkkK1ddG182hnELPxXJOEYg3lz6rHSuS241wEnP5tizyLRKZxDJbFFs28YcdPkqYjaGJ92yA9B9Smo9Gqr$ Streamline the two exascale simulation kernels..

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/cctbx/cctbx_project/pull/954*event-11462629296__;Iw!!Bt8fGhp8LhKGRg!C40sgSpkkK1ddG182hnELPxXJOEYg3lz6rHSuS241wEnP5tizyLRKZxDJbFFs28YcdPkqYjaGJ92yA9B9SD3o-JD$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AA67VEOBNQOLHJFVCDVKW7TYOBXPLAVCNFSM6AAAAABBXIU5M2VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGQ3DENRSHEZDSNQ__;!!Bt8fGhp8LhKGRg!C40sgSpkkK1ddG182hnELPxXJOEYg3lz6rHSuS241wEnP5tizyLRKZxDJbFFs28YcdPkqYjaGJ92yA9B9esWjHNC$. You are receiving this because your review was requested.Message ID: @.***>

nksauter commented 5 months ago

Mike, I've assigned you as a reviewer to get your detail-oriented review. PR#954 makes extensive and intrusive changes to the debranch_maskall kernel. There may be code conflicts with nanobragg_diffuse_aaron2 and breaking changes. I'd recommend creating a new git clone, rebasing the diffuse against exafel_kernels2 and rerunning whatever test you have to assure correct function. Once you are OK, I'll merge PR#954 and then get to work on the diffuse PR. I want to change your code so there are two options, +/- diffuse, where the minus option has no code involvement with diffuse. I believe this is a job for C++ policies and it will be the first instance using them in simtbx.

nksauter commented 5 months ago

I'm going to make a few cosmetic changes then merge. Last chance to comment. These code changes should give a big improvement in the simulation step of the KPP pipeline.