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

cupy support is not quite there #614

Closed
JTBakerAO opened this issue Mar 20, 2024 · 2 comments
Closed

cupy support is not quite there #614

JTBakerAO opened this issue Mar 20, 2024 · 2 comments

Comments

@JTBakerAO
Copy link

when i try to speed up poppy with cupy, i need to convert numpy arrays to cupy arrays first, then convert the answers back to numpy arrays to continue, say, plotting with matplotlib, etc. Some poppy plots simply fail using cupy because they don't take care of this when they use probably matplotlib wf.display(). This needs to be addressed, if possible. My fix has been to edit accel_math.py to fail on detecting cupy, then everything runs fine, except there's no cuda acceleration benefit. ):

@mperrin
Copy link
Collaborator

mperrin commented Mar 21, 2024

Hi @JTBakerAO, and welcome. I see this is your first time ever filing a GitHub issue, so let me give you some suggestions as hopefully constructive feedback. I might also suggest you see this helpful post about open source software; see their "For the User" section.

First, when filing an issue with a bug report, it's helpful to include specific details, such as example code for what you're doing that results in the bug or will trigger the issue. Concrete examples help to make it efficient for the team to be able to investigate and reproduce the issue.

Secondly, please try to keep in mind with open source software that often this work is being done gratis or pro bono, on a best-effort basis. Using dismissive phrases like "not quite there" or demanding phrases like "needs to be addressed" can come across as too demanding, for software that to be honest you're not paying for in any way. Common courtesies like "please" and "thank you" can help indicate respect and appreciation for the real human beings on the other side of the internet, who ultimately you're asking to do work as a favor to you for free...

Now, that said, let's try again. Can you provide please some specific, concrete examples of what you're trying to do that's running into trouble? Runnable code examples would be much appreciated. With that, it might be possible to take a look at this, at some point in the next while, possibly, time permitting from actual job priorities.

That said I do want to make it clear that in general, the cupy code is not intended to interact with displays much at all during the calculations. If you're optimizing for speed, one generally does not want optional graphical displays of wavefronts happening during the calculation; that slows things down. Added overhead for more transfers of data between GPU and CPY memory, and added overhead for the graphical displays. So for our use cases doing high performance calculations on GPUs, the intermediate display code paths are in general not being used. I know for a fact all the optical calculations themselves run fine with cupy; but I'm not surprised that some of the display code paths have not been fully tested for compatibility with GPUs. This is not a high priority for us, though I agree it would be nice for it to have graceful error handling or perhaps raise an exception that directly informs the user that such is not supported.

Cheers, and thanks for reaching out, and congrats on learning how to file a GitHub issue - welcome to the open source community, I hope.

@JTBakerAO
Copy link
Author

I appreciate your comments and I mean no disrespect--quite the contrary. I have since found the note "Addition of CuPy as an Accelerated Computing Option #499", which cleared up much confusion. I had installed CuPy for another purpose, but it "broke" applications that I had coded previously using PopPy. Now, later, having need of more speed in Fresnel propagation calculations, I endeavored to sort out the issues. You are correct, CuPy enhancements work fine, internally, especially when using all PopPy objects to set up optical scenarios. Functions like wf.display() will fail, as stated in note #499, which don't have the array conversion built-in to feed the internal graphics functions used. Instead, extracting the data using complex_efield=cp.asnumpy(wf.wavefront), etc., etc., allows me to utilize other calculations and plt.imshow() to work without incident. The issues I was having were related to using externally generated data in the form of numpy arrays being fed into functions when CuPy arrays were expected, whereas numpy arrays worked fine when CuPy was not installed on the system. I appreciate all of the efforts of the builders of PopPy as their algorithms are powerful, clear, stable and hence useful. I feel it has been worth the investment of my time to learn enough to fully utilize many of its features, and I encourage others daily to try it without reservation.

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

No branches or pull requests

2 participants