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

DeformableMirror performance issues #277

Open
mperrin opened this issue Dec 10, 2018 · 6 comments
Open

DeformableMirror performance issues #277

mperrin opened this issue Dec 10, 2018 · 6 comments
Labels
docs enhancement New feature or request

Comments

@mperrin
Copy link
Collaborator

mperrin commented Dec 10, 2018

Reported by @sdwill: The get_opd function can be very slow for the DeformableMirror class, in some circumstances with large numbers of actuators or high sampling.

In particular, a low-hanging fruit for efficiency is that the evaluation of the OPD should be done for just inside the DM aperture, not including all the zero padding in the wavefront.

@sdwill has volunteered to work on this.

@mperrin mperrin added the enhancement New feature or request label Dec 10, 2018
@mperrin
Copy link
Collaborator Author

mperrin commented Apr 24, 2019

@sdwill can you comment on this one? I believe the issue here is specifically with the DM class option for evaluating the surface as a sum of many gaussians, not via convolution. And the convolution path is much faster. Can you provide any more specific instance of a case in which you were encountering performance issues?

@sdwill
Copy link
Collaborator

sdwill commented May 1, 2019

Just ran some tests of ContinuousDeformableMirror on my machine with 48x48 actuators.

If no influence function is specified via the influence_function keyword, then get_opd() becomes very slow, because _get_surface_via_gaussian_influence_functions() performs a for-loop over all actuators. If an influence function is passed in during initialization, the performance appears to be much better, as expected.

It seems to me that _get_surface_via_gaussian_influence_functions() should be combined with _get_surface_via_conv() to improve efficiency when using a Gaussian influence function.

@ermaier
Copy link

ermaier commented Mar 4, 2020

I was wondering if there are any updates on this enhancement? Currently experiencing a lot of this issue in trying to implement Electric Field Conjugation with POPPY. In order to build the complex response matrix for EFC, I have to poke each actuator in my 34x34 DM and record the final complex wavefront. The first time I do this calculation, when the rest of the DM is flat, this takes ~90 seconds (using a full 28 core node on our high performance computing cluster). When I later recalculate the matrix, after there is already a shape on the DM, it takes 40-50 minutes. @douglase and I profiled my response matrix builder function and the culprit is definitely _get_surface_via_gaussian_influence_functions re-interpolating the surface all the time...

Currently trying to work around this by assigning the final derived mirror shape to the OPD of an optic that is not a DM.

@mperrin
Copy link
Collaborator Author

mperrin commented Mar 4, 2020

Ah. The trick is not to use the Gaussian influence function at all. There is another path that uses a convolution kernel with a measured influence function and that is many orders of magnitude faster. Apologies, I don’t think I realized you were using the basic Gaussian version actively and it was slowing you down. I should provide you with example code for how we’re modeling the kiloDMs in HiCAT. I’ll try to email you code tomorrow, and please remind me if I don’t.

@ermaier
Copy link

ermaier commented Mar 6, 2020

It's no problem, I didn't even realize it was an issue until last week until I started trying to recalculate the response matrix with a shape on the mirror. Good to know there's an alternative...here's a reminder :P

@mperrin
Copy link
Collaborator Author

mperrin commented Feb 4, 2021

We should update the docs for ContinuousDeformableMirror to add some brief remarks on performance, and recommendation to use the convolution pathway rather than the Gaussian.

@mperrin mperrin added the docs label Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants