moustakas
commented
2 months ago @segasai, @callendeprieto do you have any opinions regarding this ticket from the perspective of MWS? (See #276 for the original issue and this ticket for the result of @sbailey's sleuthing.)
Specifically, any thoughts on either of @sbailey's suggested solutions?
In practice I don't think this has been a big problem because normally the correct minimum is a good fit to start with and so it is only excluding non-best fits anyway. None-the-less we should consider:
- using a smaller max_velo_diff for STAR templates
- not excluding minima based upon their delta-v to a lower chi2 minimum with a bad fit. this does run the danger of racking up 3 bad fits in close proximity to each other and never getting to the correct good local minimum. But it could also be problematic to immediately discard bad fits and only keep "good" ones because we lose record that something pathological was going on in the chi2 vs. z scans and fits...
segasai
sbailey
redrock.fitz.fitz excludes chi2 vs. z minima that are within redrock.constants.max_velo_diff=1000 km/s of a previously fit lower minimum, considering it to be basically the same thing. This is too large of an exclusion for STAR templates, which only scan z=[-0.002,0.002] = [-600 km/s, +600 km/s] . Effectively we're only fitting one minimum per stellar subtype because the others would be excluded from consideration. This is especially bad if the first minimum is just wrong, e.g. this K STAR case from #276 :
The lowest point is at the edge of the scan region and gets a ZWARN=BAD_MINFIT+Z_FITLIMIT warning. But that then excludes 3 other reasonable minima as being within 1000 km/s of the bad minimum, leaving only one other minimum as the selected "good" one, even though it is clearly worse than the others.
In practice I don't think this has been a big problem because normally the correct minimum is a good fit to start with and so it is only excluding non-best fits anyway. None-the-less we should consider: