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

Minor documentation issues #117

Open
egemenimre opened this issue Oct 7, 2024 · 1 comment
Open

Minor documentation issues #117

egemenimre opened this issue Oct 7, 2024 · 1 comment

Comments

@egemenimre
Copy link

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
Copy link
Owner

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 de7641f 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

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