Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPU-related code changes, revised. #46

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

himanshugoel2797
Copy link
Contributor

@himanshugoel2797 himanshugoel2797 commented Dec 4, 2023

This incorporates most of the C++ side GPU related changes to SRW. Appears to compile fine and CPU tests pass. I think tagging of the changes should be much more thorough this time too.

Some points regarding parts which haven't been incorporated into this pull request:

  • In srwlpy.cpp is it acceptable to conditionally include auxgpu.h, or should the clients only ever reference 'srwlib.h'? This will influence how parsing GPU options passed from Python is implemented (since in the current style, when compiling for GPU, the TGPUUsageArg structure's definition is needed), thus I haven't incorporated the changes to srwlpy.cpp yet. After this I can get to ensuring that I haven't missed anything in the process of incorporating the GPU code.
  • In gmfft.cpp, I had implemented the previously unused 'howMany' parameter for 2D FFTs (the way it is already implemented for 1D FFTs), this was responsible for a lot of the changes, but is not really necessary right now. I have tried to somewhat simplify the changes in this file by reverting most of these (so the howMany parameter is once again unused for 2D FFTs), is this ok with you? or did you already review gmfft such that I should only make the minimum changes from what you provided (such that my adding the 'howMany' parameter to 2D FFTs should remain)? Depending on the decision, I still have some cleanup to do with this file.
  • Python side changes haven't been incorporated yet (eg with srwl_bl.py) since we haven't really discussed the ideal way to integrate options here (eg how to specify gpu use for only propagation or only csd). These aren't on the critical path to having an initial release, and anyway the changes are relatively small.

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.

1 participant