Make coregistration metadata consistent and public #526
Conversation
|
@adehecq @erikmannerfelt Just want to quickly check that you agree with the new names before merging. Also, are we sure about |
|
Merging to document in #502, can still make changes on your comments there before releasing |
|
|
||
| Estimates a rigid transform (rotation translation) between two DEMs. | ||
| The transform is stored in the `self.meta` key "matrix", with rotation centered on the coordinates in the key | ||
| "centroid". The translation parameters are also stored individually in the keys "shift_x", "shift_y" and "shift_z" |
There was a problem hiding this comment.
But these may be misleading, if there is a rotation, no? Should there also be an item "rotation" in the meta?
There was a problem hiding this comment.
A rotation here is quite complex (3D) and already contained in the affine matrix. Maybe we can have a note pointing at pytransform3d that has routines to convert the affine matrix into its rotation/translation/scaling components? (it's really not trivial)
We could also include the components as rotation_x, rotation_y, rotation_z.
Or just do both!
| Nuth and Kääb (2011) DEM coregistration: iterative registration of horizontal and vertical shift using slope/aspect. | ||
| Nuth and Kääb (2011) coregistration, https://doi.org/10.5194/tc-5-271-2011. | ||
|
|
||
| Estimate horizontal and vertical translations by iterative slope/aspect alignment. |
There was a problem hiding this comment.
I would write "Estimate horizontal translations iteratively using a relationship between elevation difference, slope and aspect".
Also one thing that we need to discuss/clarify. The Nuth & Kaab (2011) papers suggest to correct the vertical misalignment using the parameters of the sinusoid fit, which we don't. We use a median of the elevation difference. We should maybe add this to the description. And do we want to make the reduction function optional or we assume median is the best? The latter would be easier and is probably fine. But this means that there is no need to follow the NuthKaab method with a VerticalShift.
I'm fine with the names used. But before it was clear that the unit of offsets was pixels, because it was in the name, now it's not as clear. I'm wondering if we should have a description somewhere of the possible items in "meta". Would it make sense to have a panda's dataframe instead with a column containing the value and another containing a description? |
This PR makes the
.metaattribute a public property ofCoreg, and makes the attribute names consistent.In particular:
"offset_east_px"is now"shift_x", and is in georeferenced units (instead of pixels) which allows to remove the"resolution"key in the metadata,"offset_north_px"is now"shift_y","vshift"is now"shift_z",Others are renamed to be consistent with
BiasCorr.Will be documented in #502!
Resolves #512