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

Add gaia passbands #62

Merged
merged 11 commits into from
Jan 18, 2021
Merged

Add gaia passbands #62

merged 11 commits into from
Jan 18, 2021

Conversation

segasai
Copy link
Contributor

@segasai segasai commented Jan 18, 2021

It addresses the #40

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

Thanks @segasai.

I checked the filter curve values and it looks like they need a bit of cleaning up:

  • Some wavelengths have transmission values of 99.99. I suspect these lines should just be removed, unless there are special instructions about this in their documentation.
  • There are some transmission values < 0. These should probably be dropped (in the tails) or set to zero (in case this occurs within the filter support).
  • There are some long tails with very low transmission values. If these are consistent with zero, within the transmission measurement errors, I would drop these to avoid errors when convolving with a flux that is not specified over these long tails. It looks like a threshold around 1e-6 would be about right.
  • Each curve should have a single zero transmission point at each end, for a well defined linear interpolation model over the full filter support.

@segasai
Copy link
Contributor Author

segasai commented Jan 18, 2021

Thanks, I've fixed it. Should I put the python script that preprocess the official curves into these .ecsv somewhere in the repo ?

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

Thanks, this is the result of:

gaiadr2 = speclite.filters.load_filters('gaiadr2-*')
speclite.filters.plot_filters(gaiadr2)

gaiadr2

Can you confirm that the abrupt edge in G around 10500 is desired?

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

@segasai Can you allow me to push to your fork so I can add this plot to the sphinx docs?

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

It looks like G goes smoothly to zero around 11,000A in the plot here.

@segasai
Copy link
Contributor Author

segasai commented Jan 18, 2021

It was a consequence of me cutting by 3sigma uncertainties. I'm updating that. I think I'll do 1 sigma

@segasai
Copy link
Contributor Author

segasai commented Jan 18, 2021

I've also added you @dkirkby to my repo, so you can make commits there

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

Looks good now:
gaiadr2

I added this plot to the docs and I think everything is ready to merge now.

@dkirkby dkirkby merged commit 1a02f66 into desihub:master Jan 18, 2021
@dkirkby dkirkby deleted the gaia_filters branch January 18, 2021 15:52
@segasai
Copy link
Contributor Author

segasai commented Jan 18, 2021

Thanks for merging/help!

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

I just tagged v0.13 and pushed the new version to pypi so this can be easily incorporated into the next pipeline release.

@dkirkby
Copy link
Member

dkirkby commented Jan 18, 2021

Updated v0.13 docs are here.

@sbailey Note that this release also contains updated (DR1) DECam filters.

Thanks for the contribution @segasai!

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