Skip to content

Conversation

Deus1704
Copy link
Contributor

@Deus1704 Deus1704 commented Jun 11, 2024

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.

aia_map1 = sunpy.map.Map(sunpy.data.sample.AIA_193_CUTOUT01_IMAGE)
aia_map2 = sunpy.map.Map(sunpy.data.sample.AIA_193_CUTOUT03_IMAGE)
### Creating a template from aia_map1
bottom_left = SkyCoord(600 * u.arcsec, -500 * u.arcsec, frame=aia_map1.coordinate_frame)
top_right = SkyCoord(800 * u.arcsec, -200 * u.arcsec, frame=aia_map1.coordinate_frame)
submap = aia_map1.submap(bottom_left, top_right=top_right)

coaligned_map = coalignment(aia_map2, submap,"match_template")

Tasks:

  • Handle clipping
  • Deprecate existing API
  • Add tests
  • Add tests for checking the newly created functions
  • Add gallery examples for the new api
  • Write an exhaustive changelog
  • Decide on a better name for this method than "match_template"

@Deus1704 Deus1704 marked this pull request as draft June 11, 2024 14:43
@nabobalis
Copy link
Member

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.

@Deus1704
Copy link
Contributor Author

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.

@Deus1704 Deus1704 force-pushed the new_coalignment_api branch from 21d262d to bb4876a Compare June 12, 2024 17:34
@Deus1704
Copy link
Contributor Author

Deus1704 commented Jun 12, 2024

Should we consider returning only the shifts(pixel/arcsec) required for aligning? Like an additional argument apply_shifts = False that could be passed to the coalignment_interface and return only the shifts.
Would that be useful?

@nabobalis
Copy link
Member

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.

@kreardon
Copy link

I think optionally outputting the calculated shifts would be very useful, since it is already being calculated in this process.

@kreardon
Copy link

I also think it would make more sense to have the inputs be ordered as template_map, input_map, method
And maybe template_map isn't a great name, since there may be other, non template_matching coalignment methods that could be used?
I would call the two input maps something like reference_map and target_map

@Deus1704
Copy link
Contributor Author

I also think it would make more sense to have the inputs be ordered as template_map, input_map, method And maybe template_map isn't a great name, since there may be other, non template_matching coalignment methods that could be used? I would call the two input maps something like reference_map and target_map

Yeah, agreed. This naming and order would be better.

@nabobalis nabobalis changed the title new coalignment api (first draft) New Coalignment API Jun 14, 2024
@nabobalis
Copy link
Member

nabobalis commented Jun 14, 2024

I think optionally outputting the calculated shifts would be very useful, since it is already being calculated in this process.

Would you want the map/cube and the shifts or would you just want the shifts alone?

@kreardon
Copy link

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 x,y ordering and positive/negative sign errors in the code, but even fixing those, the registration process doesn't quite make sense.

The method employed is to use match_template to get the shift and then apply that by "clipping" the array to generate the coaligned map. But the clipping, as applied, always only works on one side of each axis (hence the weird size). Basically it seems like it is only matching the lower left or the upper right corner of the reference array, which isn't quite the expected result.

This all raises some fundamental questions:

  1. if the input arrays are different sizes (with the reference smaller), should the output map be the size of the reference or the target array?
    (my answer? for most use cases, I think output should be size of the target array suitably shifted and padded)
  2. the most common usage will likely be two arrays/maps of the same size, and in this case match_template does not work as a method (reference must be smaller). Shouldn't it work for equivalent sized arrays?
  3. the algorithm calculates the subpixel offsets, but only applies integer shifts - should interpolating to apply the sub-pixel offsets be an option (or default)?
  4. the function takes two maps as inputs, but doesn't use any of the pixel scale or alignment information to generate two arrays that should be directly compared for alignment. Instead of assuming that the plate scales are the same for both maps, shouldn't the arrays be remapped onto a common grid before the cross-correlation calculation?
  5. perhaps related to question (1), depending on how it is implemented, should it be possible to perform the shift calculation on one portion of a map/array and then apply those shift to the full map/array?
    (my answer? yes, this is a common usage scenario)

@nabobalis
Copy link
Member

I am a bit confused by what sort of "coalignment" this is doing.

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?
We want to try and have only methods in this library that are backed by publications if possible.

@kreardon
Copy link

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).

@nabobalis
Copy link
Member

nabobalis commented Jun 17, 2024

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.

about what inputs and outputs the users would like to employ with such an interface

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.

(e.g. array sizes, non-scale-matched) and the desired alignment behaviour (e.g. sub-pixel interpolation, sub-array alignment target)

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.

@nabobalis
Copy link
Member

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?
Depending on the algorithm, I can imagine the need for a range of method specific keywords but nothing we could know ahead of time for the main level.

@kreardon
Copy link

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.

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:

  1. Reference object - data array (including coordinate information?)
  2. Unaligned object - data array (including coordinate information?)
    • Unaligned object effective coverage could be bigger, smaller, or same size as Reference object?
  3. Alignment Region - (portion of Reference object to use for offset/alignment calculation
  4. Alignment Method - predefined list
  5. Aligned Object Definition - values defining output map size and coordinates

Alignment Process:

  1. Put data arrays on common coordinate grid
  2. Calculate pixel coordinate correspondence between data arrays using defined Alignment Region and Method
  3. Apply integer pixel shifts to Unaligned object
  4. Pad Unaligned object as requested in Aligned Object Definition
  5. Apply sub-pixel shifts as requested in Aligned Object Definition
  6. Map data to coordinate grid requested in Aligned Object Definition

Outputs:

  1. Aligned object (including coordinate information)
  2. Calculated image offsets (as pixel or coordinate values?)

@wtbarnes
Copy link
Member

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.

Copy link
Member

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.

@nabobalis nabobalis force-pushed the new_coalignment_api branch 6 times, most recently from cdff049 to 1f04351 Compare October 7, 2025 22:44
@nabobalis nabobalis force-pushed the new_coalignment_api branch 2 times, most recently from f0e6d29 to 179a810 Compare October 7, 2025 23:12
@nabobalis nabobalis force-pushed the new_coalignment_api branch from 179a810 to 286b3a3 Compare October 7, 2025 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a coalignment example to the gallery Refactor the coalignment module
6 participants