Skip to content

Added reverse_axis_correlation_matrix to the low-level APE 14 API - #20466

Open
astrofrog wants to merge 16 commits into
astropy:mainfrom
astrofrog:inverse-axis-correlation-matrix
Open

astrofrog wants to merge 16 commits into
astropy:mainfrom
astrofrog:inverse-axis-correlation-matrix

Conversation

@astrofrog

@astrofrog astrofrog commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Description

While chatting with @Cadair, we realised that while axis_correlation_matrix in APE 14 tells you which world coordinates depend on which pixel coordinates, there is no matrix to give the opposite.

An example that illustrates this is the solar data @Cadair pointed me to, which has:

WCSAXES =                    4 / Number of coordinate axes                      
CRPIX1  =                 80.5 / Pixel coordinate of reference point            
CRPIX2  =                416.5 / Pixel coordinate of reference point            
CRPIX3  =                 25.5 / Pixel coordinate of reference point            
CRPIX4  =                  1.0 / Pixel coordinate of reference point            
PC1_1   =       0.998246364968 / Coordinate transformation matrix element       
PC1_2   =     -0.0162493678738 / Coordinate transformation matrix element       
PC2_1   =       0.215651147459 / Coordinate transformation matrix element       
PC2_2   =       0.998246364968 / Coordinate transformation matrix element       
PC4_1   =                -5.25 / Coordinate transformation matrix element       
CDELT1  =   0.0011111111111111 / [deg] Coordinate increment at reference point  
CDELT2  =             0.000305 / [deg] Coordinate increment at reference point  
CDELT3  =           9.7517E-12 / [m] Coordinate increment at reference point    
CDELT4  =                  1.0 / [s] Coordinate increment at reference point    
CUNIT1  = 'deg'                / Units of coordinate increment and value        
CUNIT2  = 'deg'                / Units of coordinate increment and value        
CUNIT3  = 'm'                  / Units of coordinate increment and value        
CUNIT4  = 's'                  / Units of coordinate increment and value        
CTYPE1  = 'HPLN-TAN'           / Coordinate type codegnomonic projection        
CTYPE2  = 'HPLT-TAN'           / Coordinate type codegnomonic projection        
CTYPE3  = 'WAVE'               / Vacuum wavelength (linear)                     
CTYPE4  = 'UTC'                / Coordinate type code                           
...

If we load this into WCS, then axis_correlation_matrix is:

               Pixel Dim
World Dim    0    1    2    3
        0  yes  yes   no   no
        1  yes  yes   no   no
        2   no   no  yes   no
        3  yes   no   no  yes

This indicates that pixel dimensions 0 and 1 are needed to derive world coordinates 0 (lon), 1 (lat) and 3 (time).

However, for the reverse, in this case we don't actually need time to get pixel coordinates, so the matrix should look different. With this PR, we can now expose and show the matrix indicating which world coordinates each pixel coordinate needs:

Dependence of world axes on pixel axes (pixel to world):

               Pixel Dim
World Dim    0    1    2    3
        0  yes  yes   no   no
        1  yes  yes   no   no
        2   no   no  yes   no
        3  yes   no   no  yes

Dependence of pixel axes on world axes (world to pixel):

               World Dim
Pixel Dim    0    1    2    3
        0  yes  yes   no   no
        1  yes  yes   no   no
        2   no   no  yes   no
        3  yes  yes   no  yes

(this is the output from SlicedLowLevelWCS which has a nice way to print the matrices)

One subtlety is that this matrix might in some cases be degenerate, and it's not clear if we should expose just one of the matrices, or provide a way to return multiple ones in a way. However I think the cleanest path forward is to say that a given WCS instance should just provide one matrix and be consistent with what world_to_pixel_values does, and if there are multiple ways to do the WCS, then whatever is generating the WCSes could generate different WCSes with different options - it is not up to the WCS to have to worry about which one to choose. For FITS-WCS, this is not a problem in any case as the reverse matrix is always unique.

This is formally an addition to the APE 14 API, and as we stated in APE 14, the base class here is the authoritative version.

Out of scope: note that in principle, we can then modify e.g. world_to_pixel_values to be able to take None for non-useful coordinates, or provide a way to slice away useless world coordinates, but for now I am leaving these as out of scope for the present PR.

AI Disclosure

If AI tools were used to develop this pull request, describe the tools including specific model and version, how they were used, and what content is AI generated. Otherwise enter "N/A".

The original implementation was done by hand but then I used Claude to add tests, find corner cases, fix them, and implement the FITS version by adapting the axis_correlation_matrix implementation. Everything has been checked and thought about extensively by me.

  • I certify that I am human and that I take full responsibility for this pull request including all interactions with reviewers.

Merge method

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@pllim pllim added this to the v8.1.0 milestone Sep 23, 2026
@astrofrog
astrofrog requested review from Cadair and eteq September 23, 2026 14:31
@astrofrog
astrofrog marked this pull request as ready for review September 23, 2026 14:32
@astrofrog
astrofrog requested a review from mcara as a code owner September 23, 2026 14:32
@astrofrog astrofrog changed the title Added inverse_axis_correlation_matrix to the low-level APE 14 API Added reverse_axis_correlation_matrix to the low-level APE 14 API Sep 25, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants