Skip to content

ZSHTR-29 Sky emission components#9

Open
Yashvi-Sharma wants to merge 13 commits into
mainfrom
ZSHTR-29_sky
Open

ZSHTR-29 Sky emission components#9
Yashvi-Sharma wants to merge 13 commits into
mainfrom
ZSHTR-29_sky

Conversation

@Yashvi-Sharma
Copy link
Copy Markdown
Collaborator

@Yashvi-Sharma Yashvi-Sharma commented Apr 9, 2026

Added effect for getting airglow emission from PALACE and astronomical continuum emission from SkyCalc.
Added wavelength dependent Moffat PSF effect
Added AD and ADC shift effects

@Yashvi-Sharma Yashvi-Sharma requested review from baileyji and prkrtg April 9, 2026 23:19
@Yashvi-Sharma Yashvi-Sharma marked this pull request as draft April 15, 2026 23:16
@Yashvi-Sharma Yashvi-Sharma marked this pull request as ready for review April 30, 2026 18:27
Comment thread scopesim/effects/psfs/analytical.py Outdated
Comment thread scopesim/effects/psfs/analytical.py Outdated
Comment thread scopesim/effects/psfs/analytical.py Outdated
for gamma in gammas:
kernel += Moffat2DKernel(gamma=gamma, alpha=self.alpha, x_size=ksize, y_size=ksize).array
kernel /= len(gammas)
kernel /= np.sum(kernel)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will create flux if the the kernel size is too small. Since ksize is parameterized to 4x the fwhm its small and (nearly) constant, but technically the part of the sum less than unity are photons that are sent elsewhere. I'd suggest adding a check to make sure its less than .1% or some such and warning if not.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not clear to me. Can you put the check you're suggesting in the comments here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the sum of the kernel differs from the sum of the analytic, infinite extent integrals of the Moffat's by more than some small percentage then there is an issue.

ideally the kernel should be normalized by the sum of the analytic integrals. You can probably skip this step by setting the amplitude of the moffat kernel such that a pdf is returned.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the Moffat2DKernel already returns a normalized kernel that sums to 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was sorted as a move from Moffat2DKernel to ->Moffat2DModel, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, commits incoming shortly

Comment thread scopesim/effects/psfs/analytical.py Outdated
Comment thread scopesim/effects/psfs/analytical.py Outdated
return make_interp_spline(wave_array, fwhm_array) # returns bspline instance

@property
def zenith_angle(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems more like a helper than something that is a property of a PSF

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. I'll make this consistent with sky effects and remove the property tag

Copy link
Copy Markdown
Member

@baileyji baileyji May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zenith_angle should not be a method of a PSF, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll either put it in the init or make it an internal method

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that the PSF should be computing it via a helper in a util file or pulling/fetching from some global or have it passed.

Comment thread scopesim/effects/ter_curves.py
Comment thread scopesim/optics/image_plane_utils.py Outdated
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

Successfully merging this pull request may close these issues.

2 participants