-
Notifications
You must be signed in to change notification settings - Fork 10
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
DECaPS2 commit #3
Conversation
More detailed comments since a bit more was done:
|
nx, ny = im.shape | ||
|
||
# this requres stars to be "good" and in a reasonable flux range (0 flux to 17th mag) | ||
maskf = ((flags_stars==1) | (flags_stars==2097153)) & (flux_stars>0) & (flux_stars<158489.3192461114); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this 17th mag assuming some nominal zero point for a particular band? This number is oddly specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL, that is exactly what it is. It is 17th mag in g-band using the mean zero point from DECaPS 1. I don't think the actual number matters, that is just how I picked it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's clarify both these numbers after your run. i.e., 150000 to me better expresses the concept of "not too bright" than 158489; the corresponding comment might be that 17th mag in g roughly corresponds to 160k counts.
I'd usually write your flag cut as something like (flags & ~decam_proc.extrabits['diffuse']) == 2**0, but that's just stylistic. It would be good form to refer to the diffuse bit by name rather than by that decimal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Good code comments all.
This looks good to me. Feel free to merge at your convenience. "save_fxn" is starting to have few enough arguments that it might make sense to actually name its arguments and break them out of the dictionary, and give the function a real name, but I haven't actually looked at how ugly that is and will defer to your judgment. I think you have permissions to merge and should be able to do this when you're ready; let me know otherwise. Thanks! |
I tried that and had missed a value that I updated in the dictionary higher up but forgot to propagate once during the validation phase, so I am going to leave it how it is. It probably could be done, but not worth the effort. |
pyPI conforming changes (#4)
The long awaited DECaPS2 Commit. The PR includes the synthetic injection pipeline developed for DECam uncertainty estimates. Validation of the injection pipeline is available upon request.