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.

Sign up for GitHub

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

Jump to bottom

Remove really bright stars from early stages and modify offset thresholds #254

Merged
merged 7 commits into from
Feb 19, 2019
Merged

Remove really bright stars from early stages and modify offset thresholds #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5a1a779
Remove really bright stars from early stages
jeanconn Jan 30, 2019
c51a360
Add more margin to the code for bright star selection
jeanconn Feb 8, 2019
dad9fc9
Set lower bound on mag_err to 0.5 for color1=0.7 stars in bounds check
jeanconn Feb 8, 2019
c60218b
Fix long line
jeanconn Feb 8, 2019
9b51491
Leave the faint end alone for now
jeanconn Feb 8, 2019
f578d44
Add a 6th stage to bring in guide stars as bright as 5.8 mag
jeanconn Feb 17, 2019
1f2196b
Reduce offset thresholds in stages 4 and 5
jeanconn Feb 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions proseco/characteristics_guide.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
stages = [{"Stage": 1,
"SigErrMultiplier": 3,
"ASPQ1Lim": 0,
"MagLimit": [5.9, 10.2],
"MagLimit": [6.2, 10.2],
"DoBminusVcheck": 1,
"Spoiler": {
"BgPixThresh": 25,
Expand All @@ -53,7 +53,7 @@
{"Stage": 2,
"SigErrMultiplier": 2,
"ASPQ1Lim": 0,
"MagLimit": [5.9, 10.2],
"MagLimit": [6.2, 10.2],
"DoBminusVcheck": 1,
"Spoiler": {
"BgPixThresh": 25,
Expand All @@ -65,7 +65,7 @@
{"Stage": 3,
"SigErrMultiplier": 1,
"ASPQ1Lim": 10,
"MagLimit": [5.9, 10.3],
"MagLimit": [6.1, 10.3],
"DoBminusVcheck": 1,
"Spoiler": {
"BgPixThresh": 25,
Expand All @@ -77,14 +77,14 @@
{"Stage": 4,
"SigErrMultiplier": 0,
"ASPQ1Lim": 20,
"MagLimit": [5.9, 10.3],
"MagLimit": [6.0, 10.3],
"DoBminusVcheck": 1,
"Spoiler": {
"BgPixThresh": 25,
"RegionFrac": .05,
},
"Imposter": {
"CentroidOffsetLim": 2.5,
"CentroidOffsetLim": 2.0,
}},
{"Stage": 5,
"SigErrMultiplier": 0,
Expand All @@ -96,12 +96,25 @@
"RegionFrac": .05,
},
"Imposter": {
"CentroidOffsetLim": 4.5,
"CentroidOffsetLim": 3.5,
}},
{"Stage": 6,
"SigErrMultiplier": 0,
"ASPQ1Lim": 20,
"MagLimit": [5.6, 10.3],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting to 5.6 is a hack that should allow stars up to 2 * 0.1 away to be selected (5.8 brightest). Not sure where to put that in comments in the config file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't the SigErrMultiplier apply here?

Putting the comment at this line in code seems fine. I would also add a comment at the top of the stages referencing some TWiki page or section where you document all the notebooks and analysis that bears on these params.

Copy link
Contributor Author

@jeanconn jeanconn Feb 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding SigErrMultiplier, that's due to a bunch of compromises here when I thought we didn't want to let scope creep including the hacks at:

proseco/proseco/guide.py

Line 207 in ea4fa05

((cand_guides['mag'] - max(n_sigma, 1) * mag_err) < bright_lim) |

and

proseco/proseco/guide.py

Line 209 in ea4fa05

((cand_guides['mag'] - 2 * mag_err) < min(bright_lim, 5.8))

But it might make more sense to get rid of half of those compromises and not care if SigErrMultiplier is 1 for all of the later stages (which can change the spoiler status).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, though if we just set SigErrMultiplier to 1 without other changes, there's also the issue that we end up with significant catalog diffs due to it not selecting faint stars (for example a 10.25 mag star with mag_err 0.06 now could be more than 1 mag_err fainter than the 10.3 limit of the stage...).

Or could just try to redo this so it makes sense.

"DoBminusVcheck": 0,
"Spoiler": {
"BgPixThresh": 25,
"RegionFrac": .05,
},
"Imposter": {
"CentroidOffsetLim": 4.0,
},
}
]



# Guide cluster checks.
# Index is the "n_minus" value, so n - 0 stars need minimum 2500 max separation
# n - 1 stars require 1000 arcsec separation.
Expand Down
28 changes: 23 additions & 5 deletions proseco/guide.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,20 +234,38 @@ def search_stage(self, stage):

n_sigma = stage['SigErrMultiplier']

# And for bright stars, use a local mag_err that is lower bounded at 0.1
# for the mag selection.
mag_err = cand_guides['mag_err']
bright = cand_guides['mag'] < 7.0
mag_err[bright] = mag_err[bright].clip(0.1)
Copy link
Contributor Author

@jeanconn jeanconn Feb 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also figure that there's really no big issue in just bounding the effective mag_err itself for this specific check. So when combined with the stages and the SigErrMultipliers, this is effectively:

Stage 1: limit 6.2, multiplier 3, brightest possible star 6.5
Stage 2: limit 6.2, multiplier 2, brightest possible star 6.4
Stage 3: limit 6.1, multiplier 1, brightest possible star 6.2
Stage 4: limit 6.0, multiplier 1, brightest possible star 6.1
Stage 5: limit 5.9, multiplier 1, brightest possible star 6.0
Stage 6: limit 5.6, multiplier 1, brightest possible star 5.8 (the 2 multiplier applies at extreme end so this is 2 * 0.1 + 5.6

# Also set any color=0.7 stars to have lower bound mag err of 0.5
bad_color = np.isclose(cand_guides['COLOR1'], 0.7)
mag_err[bad_color] = mag_err[bad_color].clip(0.5)

# Check reasonable mag
bright_lim = stage['MagLimit'][0]
faint_lim = stage['MagLimit'][1]
bad_mag = (((cand_guides['mag'] - n_sigma * cand_guides['mag_err']) < bright_lim) |
((cand_guides['mag'] + n_sigma * cand_guides['mag_err']) > faint_lim))
# Confirm that the star mag is not outside the limits when padded by error.
# For the bright end of the check, set a lower bound to always use at least 1
# mag_err, but do not bother with this bound at the faint end of the check.
# Also explicitly confirm that the star is not within 2 * mag_err of the hard
# bright limit (which is basically 5.8, but if bright lim set to less than 5.8
# in the stage, take that).
bad_mag = (
((cand_guides['mag'] - max(n_sigma, 1) * mag_err) < bright_lim) |
((cand_guides['mag'] + n_sigma * mag_err) > faint_lim) |
((cand_guides['mag'] - 2 * mag_err) < min(bright_lim, 5.8)))
for idx in np.flatnonzero(bad_mag):
self.reject({'id': cand_guides['id'][idx],
'type': 'mag outside range',
'stage': stage['Stage'],
'bright_lim': bright_lim,
'faint_lim': faint_lim,
'cand_mag': cand_guides['mag'][idx],
'cand_mag_err_times_sigma': n_sigma * cand_guides['mag_err'][idx],
'text': f'Cand {cand_guides["id"][idx]} rejected with mag outside range for stage'})
'cand_mag_err_times_sigma': n_sigma * mag_err[idx],
'text': (f'Cand {cand_guides["id"][idx]} rejected with '
'mag outside range for stage')})
cand_guides[scol][bad_mag] += GUIDE.errs['mag range']
ok = ok & ~bad_mag

Expand Down Expand Up @@ -349,7 +367,7 @@ def get_candidates_mask(self, stars):

"""
ok = ((stars['CLASS'] == 0) &
(stars['mag'] > 5.9) &
(stars['mag'] > 5.8) &
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, had this fix local but neglected to push before your approval @taldcroft .

(stars['mag'] < 10.3) &
(stars['mag_err'] < 1.0) & # Mag err < 1.0 mag
(stars['ASPQ1'] < 20) & # Less than 1 arcsec offset from nearby spoiler
Expand Down