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 magnitude limits for Martin absolute magnitude calculation in results.py #97

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

Conversation

willcerny
Copy link
Contributor

@willcerny willcerny commented Jan 20, 2023

Update Martin absolute magnitude calculation to use magnitude limits specified in config file (rather than hardcoded values of 16 and 23.0)

Update Martin absolute magnitude calculation to use magnitude limits specified in config file (rather than hardcoded value of 23.0)
@willcerny willcerny changed the title Update results.py Update magnitude limits for Martin absolute magnitude calculation in results.py Jan 20, 2023
@kadrlica
Copy link
Member

I suspect that there was a reason this wasn't done before. I'll need to take a look at it to remember/figure out why.

@kadrlica
Copy link
Member

kadrlica commented Jun 27, 2024

Ok, so I think the reason that I didn't pass in the config magnitude limits is because absolute_magnitude_martin doesn't actually deal with these limits self-consistently, and I didn't want to make things seem more rigorous than they actually were. What actually happens in absolute_magnitude_martin is that a new isochrone is created with the same age, metallicity, distnace, but in the SDSS bandpass. Then stars are sampled from that isochrone and their V-band magnitude is calculated using the Jester et al. 2005 equations. The bright and faint magnitude limits are applied to the SDSS g-band magnitude of the stars drawn from the SDSS isochrone. This is not so bad when you are translating from DES g,r to SDSS g,r, but if you are working in DES r,i, then the magnitude limit should clearly be transformed.

Here is the code for reference:

def absolute_magnitude_martin(self, richness=1, steps=1e4, n_trials=1000, mag_bright=None, mag_faint=23., alpha=0.32, seed=None):

The original motivation for doing things this way was the fact that we only had the Jester et al. 2005 transformation to get to V-band, and so we wanted to work in the SDSS frame. I think I probably did a similar set of studies to you and concluded similarly that the exact magnitude limits didn't really matter. We now have transformations directly from DES g,r to V-band using Eqn B.8 of the DES DR2 Paper. So I think that really we should try to clean up the entire Martin magnitude calculation. Since I'm not sure anyone is really happy with the Martin magnitude calculation, this will probably mean thinking a bit about what we actually want out of this...

@kadrlica
Copy link
Member

Also see #16, #56, #57, #95. I think that probably we should take this opportunity to resolve all these issues at once, but it will take a little thought to do it "right".

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