Skip to content
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

Role of "mod_index"? #109

Open
DanRyanIrish opened this issue Jan 15, 2025 · 2 comments
Open

Role of "mod_index"? #109

DanRyanIrish opened this issue Jan 15, 2025 · 2 comments

Comments

@DanRyanIrish
Copy link

Hi. I'm trying to better understand the EISCube code-base, and was intrigued by the role of "mod_index". Is this simply to enable the WCS to be kept as a FITS-WCS object when sliced, as suggested here?

If so, there is a function in ndcube which might be helpful. It converts a sliced and/or resampled WCS back to a FITS-WCS, so long as the original WCS object was also a FITS-WCS. This could be helpful in simplifying the EISCube code base and making it easier to maintain.

But perhaps there are other uses for "mod_index" too which means it needs to be treated as it currently is?

@MJWeberg
Copy link
Collaborator

Excellent question! Yes, part of the original motivation for the mod_index was to retain a complete WCS-compatible header so we could write out a FITS file after any possible slicing. There was an issue when NDCube v2.0 was released where axis information was getting dropped, which resulted in incomplete FITS headers. I am given to understand this may no longer be the case.

We also found that maintaining a separate header dict made it easier to retain instrument-specific keywords that we wanted to export to the FITS files without modifying the original level-0 FITS header (stored in the .meta['index'] dict). This achieves a secondary goal of eispac in which we try to provide users with all of the relevant metadata to calculate the instrumental corrections themselves, in case they wish to check our calculations.

I will admit, mod_index is a somewhat awkward fix that is probably no longer strictly necessary. At the moment, it is probably still useful since we unfortunately still have a completely custom EISFitRestult sitting between the fitting step and saving the data to HDF5 or FITS (I have been meaning to try replacing this with an NDCollection, so we can keep using the great coordinate-aware functions of NDCube). For now though, it would probably be best to simply update the coordinate information from the WCS object after slicing an EISCube rather than calculating the new coords ourselves.

Thanks for raising this issue! It is a good reminder that we should strive to utilize the built-in features of NDCube rather than duplicating efforts.

@DanRyanIrish
Copy link
Author

DanRyanIrish commented Jan 17, 2025

Hi @MJWeberg. Thanks for explaining this, which makes more sense now. The ndcube function that unwraps the FITS-WCS should return at WCS with all the original axes, as well as a boolean array saying which have been dropped from the data array. With that, it should be trivial for you to reconstruct a FITS header and data array that you can put back into a FITS file. And to make it better, you would only have to do that when you wanted to save to a FITS file, rather than doing all the book-keeping for every operation.

As regards keeping the original metadata for transparency to users, NDMeta.original_header retains a copy of the original header for you. So even if values are changed at meta["key"], the original value is still available at meta.original_header["key"]. So it sounds like NDMeta fulfils another of the needs for "mod_index".

Fitting of course is beyond the scope of NDCube so, of course, I can't speak to that requirement. That said, on a related note, Astropy 7 has just released a revamp of their fitting infrastructure with solar slit spectroscopy in mind. It includes general speed-ups as well as the option to easily parallelise your fitting. Here is an example of it fitting EIS data. There's also a slightly better documented example for SPICE data. I believe it can fit ~800,000 SPICE spectra in a few minutes. Last I heard, the only drawback was that errors on the fits aren't propagated if you choose the parallel fitting option. However, there is no technical reason why it can't be. It just needs to be implemented. That's something that @Cadair and @astrofrog might like feedback on, or even help implementing. In any case, eispac might prefer its fitting infrastructure to become a wrapper around this infrastructure, given the speed-up, the support astropy can give in maintaining it, and how astropy fitting is compatible with sunpy workflows.

Finally, in the future, if there is a functionality that you need that ndcube does not appear to provide, don't feel shy to reach out. It might be that there is a way of doing what you need, or it's something we would like to support and would welcome contributions from you and eispac developers. That way you can reduce your development and maintenance burden, and/or contribute back to the wider community.

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

No branches or pull requests

2 participants