Feature branch that add PSF-related plots. #16
Conversation
LR-inaf
requested review from
jmeyers314,
jbkalmbach,
mfisherlevine and
gmegh
There was a problem hiding this comment.
This isn't a repo I should be reviewing the code for, so just leaving these as comments - you should get a review from an AOS person:
- At least in DM, people always merge from a
tickets/DM-NNNNNbranch which you'll get by filing a Jira ticket.donut_vizmay not care though, but just so you know. - You're not passing a
Figurein topsfPaneleven though you can. If you don't change that you will have a memory leak, and you won't be able to run this as part of Rapid Analysis without it crashing all the time. You just need to make a figure, hold it on the class, and reuse that though, and it should all work as long as you clear it properly between uses. - Putting all the work in the
runQuantummethod, rather than having it pass all the needed variables through to arun()method is not the normal pattern, but again, I don't know how much you want to stick to DM standards.
| filename=psf_zk_panel, | ||
| ) | ||
|
|
||
| def get_psf_degr(self, zset): |
There was a problem hiding this comment.
Can you add docstrings for all the functions?
| xs = np.array(xs) | ||
| ys = np.array(ys) | ||
| zs = np.array(zs) | ||
| psf = np.array([[self.get_psf_degr(pair) for pair in det] for det in zs]) |
There was a problem hiding this comment.
Will this work if there are not the same number of pairs in every detector? (For instance one detector didn't have enough good sources.) If this doesn't have to be an array maybe it could just be a list and that would prevent issues here.
There was a problem hiding this comment.
I see, you right. I will leave them as lists
|
Please merge to default |
LR-inaf
force-pushed
the
u/LRosignoli/psf_addon
branch
from
1d9cdf0
to
dce073c
Compare
|
Looks like the tests are failing: https://tssw-ci.lsst.org/job/LSST_Telescope-and-Site/job/donut_viz/view/change-requests/job/PR-16/4/console |
| dettype="LSSTComCam", | ||
| fig=None, | ||
| figsize=(11, 14), | ||
| maxcol=3, |
There was a problem hiding this comment.
maxcol needs an entry in the docstring
| """Run the PlotPsfZern AOS task. | ||
|
|
||
| This task create a 3x3 grid of subplots, | ||
| each axe contains the psf value for each pair of |
There was a problem hiding this comment.
each subplot contains the psf values calculated from Zernike coefficients might make it clearer where these PSF values are coming from.
| ) -> Figure: | ||
| """Make a per-detector psf scatter plot | ||
|
|
||
| Subplots shows for each detector the psf retrieve from the zernike value |
So far, a new task called PlotPsfZernTask has been implemented in the plot_aos_task.py. This task exploits an external script called psf_from_zern.py that creates the panel and gives it back to the task.