-
Notifications
You must be signed in to change notification settings - Fork 107
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
Chromatic seeing when photon shooting #1225
Conversation
@@ -221,6 +221,11 @@ def work(i, atm): | |||
significantly faster than computing PSFs with non-frozen-flow | |||
atmospheres. If ``alpha`` != 1.0, then it is required that a | |||
``time_step`` is also specified. [default: 1.0] | |||
seeing_exp: Power law index for wavelength dependent seeing when using geometric |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had talked about deriving this value from L0. Is that possible? If so, it would make it easier for users who may be using a VonKarman profile and want to get this to match up correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, my memory for how these are related was faulty. There's a whole slide deck here showing some attempts to infer a relation, but I'm not confident this can or should be used directly. Safer to just let the user specify a value I think.
@@ -1579,6 +1580,10 @@ def _shoot(self, photons, rng): | |||
u = photons.pupil_u | |||
v = photons.pupil_v | |||
t = photons.time | |||
if photons.hasAllocatedWavelengths(): | |||
w = photons.wavelength |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a test that uses this feature. (Which is the feature request that triggered this PR...) Maybe one that compares this implementation to one using ChromaticAtmosphere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I had naïvely thought that adding a WavelengthSampler
photon op to drawImage
would trigger this, but I guess those ops only get executed after the base object is drawn. Looking into how to actually trigger this now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little bit of a design conundrum even: PhaseScreenPSF
is currently a GSObject
(and not a ChromaticObject
). That's perfectly fine for the original usage, and would continue to be fine if seeing_exp = 0
. But the profile is chromatic, of course, if seeing_exp != 0
. Duck-typing might work here, but may require changing some of the code around https://github.com/GalSim-developers/GalSim/blob/main/galsim/gsobject.py#L419. For instance, the chromatic version of this profile is not separable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just be using ChromaticAtmosphere then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I figured out how to trigger this branch without a big rewrite. I had to use:
star = galsim.DeltaFunction()*sed
psf = atm.makePSF(...)
star.drawImage(..., photon_ops=[psf])
rather than
obj = galsim.Convolve(star, psf)
obj.drawImage(...)
I think it's a bit of a problem that the latter doesn't work though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... Maybe? Let me try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that does appear to be close. The main difference is that ChromaticAtmosphere applies its shift to the entire base object at once. So we couldn't specify an exponent per screen. This is probably good enough for the atmospheric screens, but won't be correct for any OpticalScreens and I don't think will be correct for the second_kick component of the atmosphere (which has a limiting case of an Airy, e.g.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do something a little custom in ChromaticAtmosphere to make it work right when the base is a PhaseScreenPSF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. That or tweak https://github.com/LSSTDESC/imSim/blob/main/imsim/atmPSF.py#L83 to build 3 separate things (atm+opt+2kick) instead of embedding them all inside a single PhaseScreenPSF. Then we can use ChromaticAtmosphere to only add chromatic seeing to the right part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latter is probably simpler...
I think we're abandoning this now, right? Since it didn't really work the way we expected. |
Adds chromatic seeing to atmospheric code for use when using geometric photon shooting (I believe similar behavior is already emergent when drawing with Fourier techniques).
To implement, I added a
wavelength
argument to the variouswavefront_gradient
methods. In retrospect, I maybe should only have added the extra argument to the private version_wavefront_gradient
, since as a purely geometric object, the real wavefront gradient isn't wavelength dependent. Adding wavelength dependence here is kind of a hack to ensure we get the lambda^-beta chromatic dependence (beta ~ 0.2-0.4) expected from the fully diffractive calculation. But I could lean either way on this one so curious for feedback.