[PIPE2D-771] Fix broken weekly Created: 11/Mar/21 Updated: 19/Mar/21 Resolved: 17/Mar/21 |
|
| Status: | Done |
| Project: | DRP 2-D Pipeline |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Story | Priority: | Normal |
| Reporter: | price | Assignee: | price |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | 2DDRP-2021 A3 |
| Reviewers: | hassan |
| Description |
|
The 2021-03-07 weekly detected a regression in wavelength calibration: ====================================================================== FAIL: testResiduals (__main__.ArcTestCase_brn_39) (arm='b', fiberId=2) Test that wavelength fit residuals are reasonable ---------------------------------------------------------------------- Traceback (most recent call last): File "/scratch/pprice/jenkins/weekly/2021-03-07/build/stack/miniconda3-4.5.12-1172c30/Linux64/pfs_pipe2d/w.2021.10/python/pfs/pipe2d/weekly/test_weekly.py", line 127, in testResiduals self.assertFloatsAlmostEqual(0.741*(uq - lq), 0.0, atol=2.0e-2) File "/scratch/pprice/jenkins/weekly/2021-03-07/build/stack/miniconda3-4.5.12-1172c30/Linux64/utils/18.1.0/python/lsst/utils/tests.py", line 735, in assertFloatsAlmostEqual testCase.assertFalse(failed, msg="\n".join(errMsg)) AssertionError: True is not false : 0.021484697109974946 != 0.0; diff=0.021484697109974946/0.021484697109974946=1.0 with rtol=2.220446049250313e-16, atol=0.02 |
| Comments |
| Comment by price [ 11/Mar/21 ] |
|
Reverting a4d7c14 ("Don't casual mess with slit offsets") in obs_pfs solved the problem. I suspect that the problem is due to the known 0.5 pixel offset in the detectorMap from the simulator. I think the thing to do is to introduce a bootstrap in the weekly to fix the 0.5 pixel offset. |
| Comment by rhl [ 11/Mar/21 ] |
|
Fixing the simulator has to be a high priority. slit offsets should be considered pretty-much sacrosanct. Running an early bootstrap to fix this would be OK too, providing the weekly itself doesn't mess with them. |
| Comment by price [ 11/Mar/21 ] |
|
I kind of like having the detectorMap from the simulator being off by a little: it mirrors reality! I therefore favour the bootstrap solution, which has the added advantage of exercising the bootstrap code to prevent bitrot. |
| Comment by price [ 16/Mar/21 ] |
|
bootstrap alone wasn't sufficient to solve the problem. I tracked the problem to ambiguous line matches, which apparently happened sufficiently frequently that it inflated even the IQR-based RMS measurements that were failing the above test. Introducing an exclusion radius of 0.1 nm (about a pixel) to the reference lines solves this, and the result is a better fit than we even had before the weekly broke (softening 0.037 pixel --> 0.022; IQR-based RMS wavelength residual per fiber ~ 0.01 nm --> 0.008; fit lines/total lines 108656/142698 --> 75229/97444; this is for the b arm). |
| Comment by hassan [ 17/Mar/21 ] |
|
Updates reviewed. No additional comments raised. Weekly test run using ticket branch was successful. |
| Comment by price [ 17/Mar/21 ] |
|
Merged to master. |