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

Change AB mag calculation to interpolate passband to spectrum resolution #7

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

BearBearCodes
Copy link
Member

This PR updates the AB magnitude calculation for a source's spectrum through a telescope's passbands.

Previously, we were interpolating the source spectrum to the passband's resolution. Instead, we should probably be interpolating the passband response curve to the source spectrum's resolution. There are two main reasons why I think we should do it this way.

  1. In most cases, the spectrum is higher resolution than our passband, and
  2. The curves in our passband files are relatively well-behaved compared to observational spectra, which may have lots of sharp peaks and troughs. If we interpolate the spectrum to the passband resolution, we risk losing these features that may have a substantial contribution to our SNR calculations. In contrast, since the passband throughputs are better behaved (i.e., smoother), any interpolation (even to a coarser spectrum) should capture the behaviour of the passband reasonably well.

If this rationale sounds right to you, @tewoods, and the changes to spectrum.py look alright, then you can approve these changes and I can merge this PR with the master branch.

@BearBearCodes BearBearCodes added the enhancement New feature or request label Jul 28, 2023
@BearBearCodes BearBearCodes requested a review from tewoods July 28, 2023 21:17
@BearBearCodes BearBearCodes self-assigned this Jul 28, 2023
@tewoods tewoods merged commit 35527e2 into master Aug 1, 2023
@BearBearCodes BearBearCodes deleted the change_ab_mag_calc branch August 13, 2023 16:59
@BearBearCodes BearBearCodes mentioned this pull request Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants