Skip to content

Fixed incorrect shift in PSF when calculating atmospheric transmission. #28

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dobos
Copy link
Member

@dobos dobos commented Apr 1, 2022

Dear @kiyoyabe , I've found this bug while debugging an issue with SNR in my updated version of the ETC. This causes a shift in the atmospheric transmission curve with respect to the sky continuum which isn't a big issue but I saw numerical differences everywhere until it got fixed. The hard-coded value of 7.5 is only correct when SP_PSF_LEN=16 but it's set to 64 by default. By the way I don't think using such a wide PSF is worth it, at least not at normal resolution.

@kiyoyabe
Copy link
Contributor

kiyoyabe commented Apr 5, 2022

Thank you for finding the issue.
Can you do the same treatment in gsGetSNR_Continuum?

/* Atmospheric transmission */
gsSpectroDist(spectro,obs,i_arm,lambda,7.5,0,SP_PSF_LEN,FR);
num = den = 0.;

Yes, we should remove the hard-coded things like:

iref = (long)floor(pos-7.5);
if (iref<0) iref=0;
if (iref>Npix-16) iref=Npix-16;
gsSpectroDist(spectro,obs,i_arm,lambda,pos-iref,0,16,FR);
for(j=0;j<16;j++)

Can you also fix this part?

64 pix is a bit large but perhaps necessary if we want to consider the wing carefully.

@kiyoyabe
Copy link
Contributor

@dobos Any comments on this? ↑

@dobos
Copy link
Member Author

dobos commented May 10, 2022

you for finding the issue.
Can you do the same treatment in gsGetSNR_Continuum?

I'll take another look once I get there. But this will be a breaking change in the sense that the output of the ETC will be numerically different. I think this tweak was added to speed up calculations with a narrow PSF because the value of 64 is overkill. I checked the differences and they're in the 1e-3 - 1e-4 range.

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