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

scopesim.Source fails with cube filename input and tabular WCS #499

Open
oczoske opened this issue Nov 12, 2024 · 7 comments
Open

scopesim.Source fails with cube filename input and tabular WCS #499

oczoske opened this issue Nov 12, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@oczoske
Copy link
Collaborator

oczoske commented Nov 12, 2024

This is from the example notebook LSS_AGN-01_preparation.ipynb in irdb/METIS, using scopesim 0.9.0a6:

src = sim.Source(cube="AGN_sim_prepared.fits")
...
File ~/ELT_Development/ScopeSim/scopesim/source/source_fields.py:283, in CubeSourceField.__post_init__(self)
    281 """Validate input."""
    282 if self.wcs is None and not self.from_hdul:
--> 283     self.wcs = WCS(self.field)
...
ValueError: HDUList is required to retrieve -TAB coordinates and/or indices.

Obviously, self.field has lost the WCS-TAB extension that is in the fits file. I don't know what self.from_hdul is, would that help?

@oczoske oczoske added the bug Something isn't working label Nov 12, 2024
@teutoburg
Copy link
Contributor

I thought I had specifically addressed TAB WCS cubes in my source fields change. Obviously not very well, at least for this case. I'll take a look (probably tomorrow), I should know where to look for this...

@teutoburg teutoburg self-assigned this Nov 12, 2024
@teutoburg teutoburg moved this from 🆕 New to 📋 Backlog in ScopeSim-development Nov 12, 2024
@teutoburg
Copy link
Contributor

I think I remember that it works if you open the file first and then pass the HDUL instead, but can't check now. Feel free to try that 🙂

@teutoburg
Copy link
Contributor

Hang on, I actually did that in this notebook! Which branch are you on for the IRDB?

@oczoske
Copy link
Collaborator Author

oczoske commented Nov 12, 2024

I guess I haven't used the latest version of the notebook. That sounds a bit hacky, though.

@teutoburg
Copy link
Contributor

Yeah I agree that filename should also work (again). I think I made this change after the whole source fields stuff broke it and this was easier than going back there. But I should implement a proper fix, it shouldn't be that hard...

@teutoburg
Copy link
Contributor

Looking at the current implementation, when Source._from_cube() (which should really be a classmethod constructor imo) receives as filename, only the extension specified by the optional ext argument is loaded, which defaults to 0. I could modify it such that loading a multi-extension FITS file will always trigger the same behavior as supplying the corresponding HDUL directly. However, that might break some code that implicitly expects that only the zeroth HDU is loaded.

My suggestion to properly solve this:

  • Allow ext=None which loads all extensions, so Source(cube="AGN_sim_prepared.fits", ext=None) would work.
  • Keep the default ext=0 to preserve backwards compatibility, but log a warning that this will change if no ext is supplied.
  • Change the default to ext=None in the next minor version and explicitly supply ext=0 in all the cases where that's what we actually want in our own code and notebooks.

@teutoburg
Copy link
Contributor

teutoburg commented Nov 16, 2024

Addendum: It is possible to load a TAB WCS from another HDU even with a certain ext set, see the current implementation of CubeSourceField.from_hdulist(). So we might actually be fine with keeping the default and just always treat the filename case identically to the HDUL case. After all, even with a TAB WCS, ScopeSim still needs to know in which HDU the actual cube data is. So maybe what I wrote in the comment above isn't such a good idea after all...

@teutoburg teutoburg changed the title scopesim.Source fails with cube input and tabular WCS scopesim.Source fails with cube filename input and tabular WCS Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants