brandondube / prysm

physical optics: integrated modeling, phase retrieval, segmented systems, polynomials and fitting, sequential raytracing...
https://prysm.readthedocs.io/en/stable/
MIT License
267 stars 46 forks source link

Minor documentation issues #117

Open egemenimre opened 1 month ago

egemenimre commented 1 month ago

Just a couple of fixes needed with the documents:

  1. The equation for peak=1 at the end does not seem to match the equation in the code. link

  2. No PSF is seen in Block 7 and 8. Is this normal? link

  3. Generally I would recommend using radius = diameter / 2 in the examples rather than writing 10 and 5 within the equations. It would make following things much easier. In fact, I would suggest not using any values in the calls, such as wf.focus(100). This would be way easier to follow: wf.focus(efl).

  4. In Block 1, the comment says "10 waves of defocus", though the code seems to say 15. link

  5. Is padding necessary in Block 4? link

  6. The link at the end of Grids section is broken. link

  7. While not a fix, a tutorial about degradations like jitter and smear would be appreciated. I feel like there is so much more in the library that is buried in the API description. :)

brandondube commented 1 month ago

Thanks for the report. As a general remark, the last stable release is quite a long time ago -- the next one has been "any day now" for ~6 mo+, but I have been quite busy with work and not had the time to do the last polish pass before cutting a release. Anyway, that is to say that unless you are using the last released version, things may already be fixed in the current master tree. I put a few notes in the same numbering you used:

  1. Thanks, this was actually already fixed, but I just pushed de7641fa83adfe0ee8c0b4e116e04306c8041716 which touches up the grammar a bit more

  2. No, this is a carry-over from when I updated the radiometric normalizations in the last release and forgot to update that doc, but it is already fixed on the master branch

  3. Pull requests accepted to improve documentation =)

  4. Pull requests accepted :)

  5. Doing the padding before the normalization is required, yes -- the pull I linked above touches up the 'why.' If you will always use certain simple factors of Q (like 2, or 3, or any integer) then you can just do np.sqrt(aperture.size*(Q*Q)). For noninteger Q, you need to get the rounding just so and it is better to just pad and sure you got it right

  6. Thanks -- that's one of the bits of documentation that didn't survive one of the past great big rewrites and I missed that re-doing the docs. I'll remake that... eventually, which also addresses your point 7