tickets/DM-39836: Change the algorithm FitAffineWcsTask uses #183
Conversation
|
|
||
| # Create containers for the data going into the Affine fit (i.e. | ||
| # solving Ax=b where x will be the affine parameters and a linear | ||
| # shift. |
There was a problem hiding this comment.
Missing close-).
This would also be a good place to document how you've ordered the affine transform parameters when flattening the matrix and offset into a 6-element vector.
And to be super precise, instead of just saying "solving Ax=b" you might want to say "minimizing ||Ax - b||" or otherwise referencing least-squares.
| A = np.zeros((len(matches)*2, 6), dtype=float) | ||
| b = np.empty(len(matches)*2, dtype=float) | ||
|
|
||
| # Constant terms related to the shit in and and y parameters |
There was a problem hiding this comment.
| # Constant terms related to the shit in and and y parameters | |
| # Constant terms related to the shift in x and y parameters |
|
|
||
| # Constant terms related to the shit in and and y parameters | ||
| A[:len(matches)*2:2, 4] = 1 | ||
| A[1:len(matches)*2+1:2, 5] = 1 |
There was a problem hiding this comment.
I think
A[::2, 4] = 1
A[1::2, 5] = 1
is equivalent to the above, and more readable.
There was a problem hiding this comment.
I agree. Those were left from previous slicing operations when I had things arranged differently and never cleaned them up. Thanks for the detailed reading
| gtol=2.31e-16, | ||
| xtol=2.3e-16) | ||
| self.log.debug("Best fit: Direction: %.3f, Dist: %.3f, " | ||
| val = forward.applyForward(srcCentroid) |
There was a problem hiding this comment.
I'm pretty sure we could get another speedup by vectorizing these calls to applyForward if the profile says that'd help. The usefulness of that is limited by the fact that we're starting with a ReferenceMatchVector rather than something with array-column coordinates, though.
There was a problem hiding this comment.
Yeah I thought about this too. Most of the time seems to be "within this function" so I suspect it is things like this. However it was so much faster I didn't know if it was worth opening up another repo and adding an interface which is pretty much just for this. I would be happy to do so if we thought it worth it.
There was a problem hiding this comment.
Let's just get this in. Doing more could easily take a lot longer than this ticket has so far.
This commit changes FitAffineWcsTask to directly solve for affine and shift parameters by solving a matrix equation instead of creating wcs objects while iterating in a solver. This also absorbs linear shifts inside the linear transforms instead of solving for a sphere point offset in the initial wcs.
natelust
force-pushed
the
tickets/DM-39836
branch
from
ee9b2fb
to
6278c24
Compare
This commit changes FitAffineWcsTask to directly solve for affine and shift parameters by solving a matrix equation instead of creating wcs objects while iterating in a solver. This also absorbs linear shifts inside the linear transforms instead of solving for a sphere point offset in the initial wcs.