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

Update tsnr.py #1815

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

Update tsnr.py #1815

wants to merge 1 commit into from

Conversation

callendeprieto
Copy link

Changed nominal flux for backup and gpbbackup in var_tracer from 27.5 nMgy (r=18.9 mag star) to 397 nMgy (r=16 mag star).
We would like to change the effectime for the backup program from 60 to 90 seconds.

changed nominal flux for backup  and gpbbackup in var_tracer from 27.5 nMgy (r=18.9 mag star) to 397 nMgy (r=16 mag star)
@schlafly
Copy link
Contributor

schlafly commented Aug 2, 2022

This looks good. Two things:

  1. We'll need to discuss how to apply this change to all past backup observations.
  2. Somewhere there's a coefficient setting efftime = coeff * tsnr. Have you encountered that? That coefficient may need to be updated. Ultimately we measure completeness in efftime so that needs to be correctly calibrated if we want to get the right exposure times.

@callendeprieto
Copy link
Author

callendeprieto commented Aug 4, 2022 via email

@schlafly
Copy link
Contributor

schlafly commented Aug 8, 2022

We'll probably have to re-compute efftimes for all past tiles once this code goes in. That's fine; I'm just trying to make sure we don't forget to do that. I'll bring this up on the data call tomorrow, and see if @julienguy or @araichoor have any memory of how the backup program tsnr2 -> efftime relationship is established.

@weaverba137
Copy link
Member

@schlafly, @sbailey, what would be the consequences of merging this PR now?

@sbailey
Copy link
Contributor

sbailey commented May 17, 2024

Short version from my perspective: I don't know the impact, but given that we've been living without it for several years it can't be too urgent, and I'm inclined to not change things that aren't required when we're imminent on going back on sky after an instrument disruption. If this is an important feature to get into Jura before the final catalogs are made, we need to hear the case for that.

@schlafly
Copy link
Contributor

This is not an important feature for Jura. Before we would put this in we would need to revisit the backup program tsnr -> efftime mapping, which we didn't do and so this PR got stuck.

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.

4 participants