-
Notifications
You must be signed in to change notification settings - Fork 106
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
Kolmogorov atmosphere #105
Comments
Sounds like a plan, thanks! |
The increasingly epic discussion on issue #117 mentions image scaling, I am trying to implement a simple long-exposure Kolmogorov PSF (#105) SBProfile has a stepK() method. This has a different value than the |
Hi Joerg, I know you were interested in a description of what
So the stepK is indeed the k-space step size, however I cannot find the
In short, no. The pixel scale can be chosen by the user. Unless the user requests another pixel scale, the sensible choice of pixel scale in real space I think the code generally defaults to the Nyquist value In the crazy inverted world of Fourier-real mappings, Hmmm. Not very concise! I'd be happy to chat on Skype about this if you wish Joerg (will email my Skype name if that would be handy)! |
This is very helpful! Thank you very much, indeed! A lof of the -- Joerg Jörg Dietrich jorgd@umich.edu http://www-personal.umich.edu/~jorgd/ |
Ah ha, this suggests we should make sure that useful information from SBProfile.pdf ends up in the DOxygen docs. |
I've added this to the remit of #21, good suggestion. |
I have removed this from Milestone 2. I won't be able to finish it on time and it's not holding anything up right now, as far as I can tell. I'll get to work on it next week. |
You're right that it's not holding anything up, so don't worry about it. |
Fine by me, thanks Joerg (and hope Munich is nice!) |
I'm beginning to feel incredibly stupid and frustrated by this. I still don't understand what the interplay of the various parameters is. Why is the maxk of OpticalPSF different than the one reported by maxK()? There's now some code outline in this branch. Why is the shape of .draw() different than (npix, npix) if dx=1? |
Perhaps the best thing to do for efficiency is: i) Joerg, you and I talk on Skype today/tomorrow/as soon as is convenient ii) Based on the places where we find my code has confused you so badly My skype username is barnaby_rowe... When would be good for you? (Anyone On Tue, May 22, 2012 at 9:04 AM, Jörg Dietrich <
Barnaby Rowe Jet Propulsion Laboratory Department of Physics & Astronomy |
Tomorrow at 2pm EDT? skype id joergpdietrich |
Joerg, I've just discovered this issue and can offer a little potential help since I had implemented a Kolmogorov PSF in an earlier version of the SBProfile code. Here is a snippet of what I had written:
The defining characteristic of Kolmogorov is that its Fourier Transform is a power law, exp(-pow(k/k0,1.666667)), where FHWM = 2.92/k0. You'll want to set kMax to the point where you're willing to neglect any higher k's. So one way to do it would be to solve
which for ALIAS_THRESHOLD=0.005 gives maxK = 2.7 k0. (In the code above this was hard-wired to 3*k0). For stepK, you need to decide at what radius you'd be willing to "fold" the PSF onto itself. Then set stepK=pi / (fold radius). Above I had chosen to always go to at least radius 5/k0 = FWHM * (5/2.92). The hard part about Kolmogorov is that the real-space radial function is not analytic. The Moffat approximation is nice, but it's not exact - looks like errors as big as 1% of peak in that Trujillo paper you reference. The other SB's have pretty high accuracy equivalence between their real-space and Fourier results, which we might wish to maintain. So we may wish to compute the real space Kolmogorov via DFT from the formula above. It would also be ok to leave the real-space function "unimplemented" so the DFT will simply be done on demand. I'm happy to help as needed... |
2pm EDT tomorrow works fine for me Joerg, thank you. And Gary: that's great, thanks, very useful... I really want to get involved in the atmosphere stuff more myself now too, it interests me! |
Hi Joerg, So following our very helpful conversation, I've opened an issue about the clarity of the OpticalPSF kmax stuff (#166). However, before I open an issue about the |
Aha, so in fact Joerg now I look I do see that the scale of the BaseImage is set in the constructor in Image.h, and it's derived subclass constructors that follow this pattern, whereas it wasn't before. Someone must have spotted this. However, the setting of As a sanity check, would it be possible to |
There was some screwiness in |
hehe simultaneous problem-solving redundancy :) |
No change:
|
can you use On May 23, 2012, at 3:51 PM, Jörg Dietrich wrote:
Rachel Mandelbaum |
Maybe this should go to its own issue now? |
Now, I think we need to use |
hmmm, think I've found it. will report back after lunch before opening the issue, would like to understand this! |
OK, I think I understand what is going on, and I don't think it's wholly a bug, but will explain (N.B. If @gbernstein or @rmjarvis are able to point to flaws in the following logic, I'd be grateful!) When the
...using the maxK() of the SBProfile. For Specifically, the line that sets this is 82 in SBInterpolatedImage.h
The urange methods return different things for different interpolants. For the Sinc function interpolation, urange() = 0.5, giving the classic Nyquist-Shannon result. For the Lanczos, in this case the Lanczos5, there is actually a bit of calculation to be done using the cubic spline approximation to the Lanczos in Fourier space that SBProfile uses... The end result depends on the
We can see what gets chosen for your Lanczos5 with tol=1.e-4 by looking at the SBProfile attribute in Python:
then
Since This means that the default Finally, as a sanity check, 266 / dx = 993.776, which rounds up to 994 as you found. So I think the drawing at least is behaving how we expect. However... I do think it would have been nice if this value of
...the This is a question again aimed I guess at Gary and Mike: is there a way to make sure this image has the correct |
Your explanation of the genealogy of the dx used in the FFT looks correct to me. But I don't understand your final question: what do you mean by the "correct" dx? Do you want the output Image() class's scale value to match the dx chosen by the FFT? Or do you want the image to be drawn with a preselected dx? I thought the code did both already in fact, the latter when you call draw() with a positive value of dx. On May 23, 2012, at 6:22 PM, Barnaby Rowe wrote:
|
The issue is that, when drawn using the default-chosen
when in the physcial units used throughout the |
True enough. But it looks like the setScale is called there too. On May 23, 2012, at 7:29 PM, Barnaby Rowe wrote:
|
Hmmm... Yes, it gets set by what comes out of the
I found this in src/SBInterpolatedImage.cpp:
Is it possible that the first case is being switched into above, since I don't see a |
Yes, I think you've got it: that override does not call setScale in the first branch. On May 23, 2012, at 7:47 PM, Barnaby Rowe wrote:
|
OK, going to test there, will let you know (didn't see much in the Python!) |
Yes! That fixes it:
Joerg, are you happy with this default behaviour? |
(If so, I might make this one line fix on master and document it there with a comment that refers to this issue). |
I agree about fixing on master. On May 23, 2012, at 8:27 PM, Barnaby Rowe wrote:
Rachel Mandelbaum |
(not sure I like that default behavior, but that can be hashed out here, whereas the bug fix should go on master once we make sure it doesn't break some test which we should also fix.) |
It doesn't break scons, scons examples or scons tests so far. I'll try running the demos... |
Sounds like you're probably okay then. But... we sould think about the fact that we didn't have a unit test that uncovered this! On May 23, 2012, at 8:40 PM, Barnaby Rowe wrote:
Rachel Mandelbaum |
Runs on demos on this branch. Will put on master and comment (have to run home now). For the unit tests, what do you think? Store values of dx that we know are right (e.g. the one above) for various interpolants to use as regression tests? Or simply check for dx < 1 (for all non-sinc interpolants) on the basis that we can't do better than sinc? Struck me from the discusion above, this behaviour (i.e. the adding of a kernel interpolant's extended k-space support to maxK, whether or not this is then used to set the default dx) is non-trivial to describe, we'll have to make sure the documentation is good on this! |
I think it would make sense to start a new issue for this question of unit tests, as others who have played with the code and might not be following this issue could have some useful ideas. I agree with you about documentation. |
I think I like this behavior. On Wed, May 23, 2012 at 05:27:32PM -0700, Barnaby Rowe wrote:
Jörg Dietrich jorgd@umich.edu http://www-personal.umich.edu/~jorgd/ |
…turning filled image - was wrongly using default=1 - to SBInterpolatedImage draw() method when no dx is specified by the user. See Issue #105.
Hi all. I merged the pull request for this branch, closing the issue. Many thanks! |
So as mentioned above, I merged with master and very stupidly didn't do last checks. We're getting this test failure:
Currently this means that some tests are failing on master, apologies again! |
I went ahead and fixed this on master. There was also a python 2.6 incompatibility (tuple - tuple) which I fixed. I don't think this merits another pull request. |
Great, thanks Mike! |
Implement an atmospheric PSF based on Kolmogorov turbulence. I'm assigning this to myself and plan to have it ready for the 2nd milestone. This will be done in python initially and may have to be rewritten in C++ if it is too slow, as discussed in #25.
The text was updated successfully, but these errors were encountered: