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

DES references #42

Merged
merged 4 commits into from
Jan 13, 2022
Merged

DES references #42

merged 4 commits into from
Jan 13, 2022

Conversation

mpaillassa
Copy link
Collaborator

I am starting this PR to update the DES survey even though the HSC PR #32 is not yet merged as we are waiting for the last HSC obscuration details.

Note that the CTIO website URL has recently changed from ctio.noao.edu to ctio.noirlab.edu so that the main DES survey parameter reference links in the corresponding issues are dead.

For the filter parameters, I have chosen to use the DR1 paper as it contains the sky_brightness, exp_time and psf_fwhm in the same place (we use speclite for zeropoints). We may use the DR3 values but the sky brightness is given in e-/pixel so it would need to be converted to mag/arcsec^2.
Also, in practice, exposure times are 90s (for DR1) and those exposures are coadded by 10 to make 900s coadds so I am not sure if we should set 90s or 900s as exposure time. As previous values were about 800-1000s, I assumed we should use 900s.

@ismael-mendoza
Copy link
Contributor

Hi @aboucaud and @mpaillassa - do you also want to include the central_wavelength in every filter of each survey? Currently only "Rubin" has this specified.

@mpaillassa
Copy link
Collaborator Author

Hi @ismael-mendoza. Yes, I remember that we use the same Rubin/LSST central wavelength for all surveys in BTK but it would be nice to have a central wavelength for each specific survey filter. I guess this is to be managed with #36.

For DES we can find information here. Especially, I guess we can compute the central wavelength thanks to the Excel spreadsheet but what is the exact definition of the central wavelength ? Is it the filter response weighted sum of wavelengths through the bandpass ?

@ismael-mendoza
Copy link
Contributor

Thanks @mpaillassa for the info, I'll talk to Javi about how they computed it in WeakLensingDeblending and let you know. The relevant line is here but the link mentioned is broken.

Feel free to proceed with this PR if possible without the central_wavelength information and we will manage it in a different PR along #36

@mpaillassa
Copy link
Collaborator Author

I just removed the bad comments as we've just discussed. If it is ok for you @aboucaud, we can merge it and leave the central wavelength for another PR ?

Copy link
Collaborator

@aboucaud aboucaud left a comment

Choose a reason for hiding this comment

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

Looking good !

I find it very strange that the PSF FWHM values were that much off..
Thanks for seeing this through.

@aboucaud
Copy link
Collaborator

@mpaillassa are you ok with resolving the existing conflicts or do you want me to do it ?

@mpaillassa
Copy link
Collaborator Author

@aboucaud It's ok I just did it.

galcheat/data/DES.yaml Outdated Show resolved Hide resolved
@aboucaud
Copy link
Collaborator

Thanks a lot @mpaillassa !

@aboucaud aboucaud merged commit def5682 into LSSTDESC:main Jan 13, 2022
@mpaillassa mpaillassa deleted the add_DES_reference branch January 13, 2022 12:02
aboucaud pushed a commit that referenced this pull request Feb 9, 2022
aboucaud added a commit that referenced this pull request Feb 13, 2022
* Add data README

* Add file to package data

* Add link to main README

* Address Maxime comment

* Update DES references (#42)

* Adding missing gain values (#44)

* Adding missing obscuration values (#46)

* Rename exp_time to exposure_time (#47)

* Update pre-commit

* Update sky brightness definition

* Fix exposure_time name

* CFHT references (#51)

* first rework
* cfhtls exposure times
* renaming to cfhtls

* Include the effective wavelength

* Add ismael's remarks

Co-authored-by: mpaillassa <[email protected]>
Co-authored-by: mpaillassa <[email protected]>
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.

3 participants