-
-
Notifications
You must be signed in to change notification settings - Fork 47
New Coalignment API #207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
New Coalignment API #207
Conversation
I have a broader comment. Can we not touch the original code but create a new coalignment folder and have separate files for the interface and each method? This would make it easier to prototype and review for others. |
That works. Let me get that done quickly. |
21d262d
to
bb4876a
Compare
Should we consider returning only the shifts(pixel/arcsec) required for aligning? Like an additional argument |
On the surface, I would rather return the shifted maps. It might be useful information to have tho but maybe I would want that to be a seperate function. |
I think optionally outputting the calculated shifts would be very useful, since it is already being calculated in this process. |
I also think it would make more sense to have the inputs be ordered as |
Yeah, agreed. This naming and order would be better. |
Would you want the map/cube and the shifts or would you just want the shifts alone? |
I am a bit confused by what sort of "coalignment" this is doing. In the above example, the submap reference is [500, 335] pixels, while the target aia_map2 is [1345, 1217] pixels. The coaligned_map is [956, 1047] pixels and, as can be seen in example images, isn't really coaligned with the reference. There are some The method employed is to use This all raises some fundamental questions:
|
Ideally when we have the interface in place, we can either re-write this method to be useful or we remove it and implement something new. I am not too worried about the fact that this really old code from sunpy isn't actually good. Are you aware of any papers about co-alignment methods we could look at implementing here? |
Thanks, Nabil. I realize the actual alignment code is just one implementation. But I think my questions were mostly non-implementation-specific, about what inputs and outputs the users would like to employ with such an interface (e.g. array sizes, non-scale-matched) and the desired alignment behaviour (e.g. sub-pixel interpolation, sub-array alignment target). |
Ah I see, I apologize, I thought it was angled at the really old/bad implementation.
I think this is a topic we want to get as many user feedback as possible. If you happen to know others who have thoughts, I would love to hear it as well.
Naively, I would assume that is method dependent? Would users tweak these kind of behaviours as keywords typically or would that be method specific keywords? I will say this as a person who hasn't had to coalign data before and I am not familiar with current methods. |
So some basic tests for the match template would be good, I would hope the original ones would work. On a different note. Do we want the ability for the user to pass keywords arguments to the method? |
Yes, I think a map-aware co-alignment process could be useful for many users. The added layer of complexity is automatically dealing with two maps which don't share the same image scaling (as well as higher-order differences like rotation?). One could think of aligning an IRIS slitjaw image and an AIA 1600 image as a useful example. Do we want this API to apply the appropriate scaling before calculating the coalignment offsets? Or should the API only deal with data arrays that have already been mapped to a common grid through some other procedure? Things get more complicated when thinking about the rarer (corner) case of aligning SolO/EUI and SDO/AIA images taken from different viewing angles. Sometimes these can be appropriately aligned if the two maps are on a common physical scale (km at solar surface/pixel), other times it might make more sense to reproject the images to a common viewing geometry. So trying to sketch out the inputs and outputs of such an API, here is what I came up with (including some question marks for options that may depend on implementation): Inputs:
Alignment Process:
Outputs:
|
I just pushed a half-baked, in-progress attempt at enforcing consistency in ordering and naming of the arrays. There are still a few fixes to be made so hold off on any reviews, tweaks, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this page still needs a bit of tweaking before this gets merged.
cdff049
to
1f04351
Compare
f0e6d29
to
179a810
Compare
179a810
to
286b3a3
Compare
Fixes #83
Fixes #103
Following our discussions, in this pr I've laid the skeleton for the new coalignment interface.
A basic use case is demonstrated, where the match_template method is registered by default. For details on the registration process, please refer to the code.
Tasks: