Check SQuaSH metric regressions using Match PessimisticB

XMLWordPrintable

Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
4
• Sprint:
AP S19-3
• Team:

Description

After the new matcher (Pessimistic B) became default on DM-14857, the astrometric metrics recorded on SQuaSH regressed (for both HSC and CFHT). Reproduce this locally and understand what happened. Fix if this is a matcher problem; if it's a validate_drp problem, ticket that for future work.

Activity

Hide
Chris Morrison added a comment -

I've updated the comment in the ticket. I'm waiting on either John Swinbank or yourself, Yusra AlSayyad to give it a thumbs up. I'm ready to merge once that happens.

Show
Chris Morrison added a comment - I've updated the comment in the ticket. I'm waiting on either John Swinbank or yourself, Yusra AlSayyad to give it a thumbs up. I'm ready to merge once that happens.
Hide
John Swinbank added a comment - - edited

Chris Morrison — a couple of things:

• Could you take another pass through the commit messages? I still see an “interative” there, as well as a couple of “explaination”s.
• I actually think the comment you've written here on this ticket is more informative than the one you added to the source. Do you think you could adapt it? Something like:

The shift/rotation matrix returned by _construct_pattern_and_shift_rot_matrix, above, was based on only six points. Here, we refine that result by using all of the good matches from the “final verification” step, above. This will produce a more consistent result.

perhaps?

Show
John Swinbank added a comment - - edited Chris Morrison — a couple of things: Could you take another pass through the commit messages? I still see an “interative” there, as well as a couple of “explaination”s. I actually think the comment you've written here on this ticket is more informative than the one you added to the source. Do you think you could adapt it? Something like: The shift/rotation matrix returned by _construct_pattern_and_shift_rot_matrix, above, was based on only six points. Here, we refine that result by using all of the good matches from the “final verification” step, above. This will produce a more consistent result. perhaps?
Hide
Chris Morrison added a comment -

Done, sorry I forgot about the typo in the commit. I double checked and the Cfht values are the same as I reported above. Let me know when I'm clear to merge.

Show
Chris Morrison added a comment - Done, sorry I forgot about the typo in the commit. I double checked and the Cfht values are the same as I reported above. Let me know when I'm clear to merge.
Hide
John Swinbank added a comment -

. Go for it.

Show
John Swinbank added a comment - . Go for it.
Hide
Chris Morrison added a comment -

Fingers crosses that the squash metrics improve as I have seen. Talked to Simon Krughoff this morning and he said that squash will run with this version tomorrow morning.

Show
Chris Morrison added a comment - Fingers crosses that the squash metrics improve as I have seen. Talked to Simon Krughoff this morning and he said that squash will run with this version tomorrow morning.

People

Assignee:
Chris Morrison
Reporter:
John Swinbank
Reviewers:
Watchers:
Chris Morrison, John Swinbank, Yusra AlSayyad