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

SDSS filters missing atmospheric transmission factor? #66

Closed
tloredo opened this issue Apr 30, 2021 · 8 comments
Closed

SDSS filters missing atmospheric transmission factor? #66

tloredo opened this issue Apr 30, 2021 · 8 comments

Comments

@tloredo
Copy link

tloredo commented Apr 30, 2021

Hi specliters-

In trying to move to speclite from some quick-and-dirty synthetic photometry code of my own, using SDSS filters, I found that I could get my calculations to agree w/ speclite only by dropping the atmospheric transmission factor from the SDSS filter responses in my calculations.

The speclite documentation indicates that its SDSS filter responses are "calculated as the reference response multiplied by the reference APO atmospheric transmission": https://speclite.readthedocs.io/en/stable/filters.html.

The filter data tables have comments indicating the same. E.g., for the u band, a comment reads, "Values are the product of the u and atmospheric transmission columns from Table 4 of Doi...": https://github.com/desihub/speclite/blob/v0.13/speclite/data/filters/sdss2010-u.ecsv

Here are the first few entries in the speclite SDSS u band filter data file:

wavelength response
2930.0 0.0
2940.0 0.0001
2960.0 0.0003
2980.0 0.0005
3000.0 0.0009
3020.0 0.0014
3040.0 0.0029

Here is the content at the start of the Doi+ Table 4 (ASCII version here: https://iopscience.iop.org/1538-3881/139/4/1628/suppdata/aj330387t4_ascii.txt?doi=10.1088/0004-6256/139/4/1628):

lambda	u	g	r	i	z	T_1.3 air mass	
2940	0.0001	 ... 	 ... 	 ... 	 ... 	0.0162	
2960	0.0003	 ... 	 ... 	 ... 	 ... 	0.0242	
2980	0.0005	 ... 	 ... 	 ... 	 ... 	0.0351	
3000	0.0009	 ... 	 ... 	 ... 	 ... 	0.0459	
3020	0.0014	 ... 	 ... 	 ... 	 ... 	0.0644	
3040	0.0029	 ... 	 ... 	 ... 	 ... 	0.0828	

I checked a couple other SDSS data files, with similar findings. It appears the speclite SDSS filter responses do not include the atmospheric transmission factor.

Have I misunderstood something here?

@dkirkby
Copy link
Member

dkirkby commented May 4, 2021

Thanks for reporting this @tloredo. Looking back at my correspondence with Mamoru Doi, I see that I was confused about whether the atmosphere was already included in the Table 4 values (despite the fact that it is obvious from the curves).

At this point, I think the simplest fix is to update the metadata for the existing curves to reflect the fact that there is no atmospheric extinction included.

Would it then be useful to add new curves with the Table 4 reference atmosphere included?

@dkirkby
Copy link
Member

dkirkby commented Sep 9, 2021

I just updated the SDSS filter curve metadata to clarify that no atmosphere is included in these curves.

Closing this now, but feel free to add a new issue if you need a separate set of curves with the X=1.3 atmosphere included.

@dkirkby dkirkby closed this as completed Sep 9, 2021
@tloredo
Copy link
Author

tloredo commented Oct 22, 2021

Hi David. Just a quick follow-up. speclite came up today in a Rubin DP0 discussion, where we had a quick look at the docs in a Zoom breakout. I noticed that the docs at ReadTheDocs still indicate that the SDSS curves include atmospheric transmission. So it looks like the docs need a quick update.

@dkirkby
Copy link
Member

dkirkby commented Oct 25, 2021

Thanks @tloredo. The ECSV metadata is already updated (e.g. here) and I am re-opening this issue to update the sphinx docs as well.

@dkirkby dkirkby reopened this Oct 25, 2021
@moustakas
Copy link
Member

@dkirkby would it be possible to add a new set of SDSS filter curves (and arguably make those the default) which include the atmosphere. I'm doing some comparisons with K-correct and only just realized that the discrepancies I'm seeing with my code are likely due to this issue.

@moustakas
Copy link
Member

Also note that this ticket is a duplicate of #39.

@andreufont
Copy link

andreufont commented Jul 12, 2022 via email

@moustakas
Copy link
Member

Done in #76.

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

4 participants