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

Generation of Chandra 'photometric' products #1264

Open
DavidT3 opened this issue Nov 20, 2024 · 8 comments
Open

Generation of Chandra 'photometric' products #1264

DavidT3 opened this issue Nov 20, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request telescope specific Changes specifically for one telescope supported by XGA, rather than the overall infrastructure

Comments

@DavidT3
Copy link
Owner

DavidT3 commented Nov 20, 2024

This part of the project deals with the implementation of XGA wrappers for the CIAO routine(s?) required to make 'photometric' data products - note that 'photometric' isn't really a term generally applied to X-ray data, but I like to be able to make a distinction between things like images and things like spectra.

For this project task we'll want to generate (all in user-configurable energy bands):

  • Images
  • Straightforward effective exposure maps in units of seconds
  • Flux maps (though they don't have a product class to be put into yet).
  • Maybe count-rate maps? But maybe we'll just let XGA do its normal thing of dividing images by exposure maps.

While fluximage is extremely useful, it has the downside that a run of it will produce multiple different files/product types, so that means that the 'execute_cmd' function (common to all telescopes;

if p_type == "image":
) will have to be modified in some way to account for that.

For other telescopes we have had separate functions for generating each product, but using fluximage is going to be a time saver so I think we'll just live with having one function make multiple products.

You're going to implement two photometric generating functions; the first will be akin to the 'rate' mode in the DAXA wrapper for flux-image (https://github.com/DavidT3/DAXA/blob/853954ed7a839398747e0254f04ef077f9538562/daxa/process/chandra/generate.py#L26) - this will generate images, 'normal, exposure maps, and rate maps (though again they might be discarded in favour of how XGA already does it.

The second will be akin to the 'flux' mode in the DAXA wrapper, and will make flux maps (also weighted exposure maps but maybe we won't keep them).

Start by setting up a generate.ciao.phot part of the module, then make a 'chandra_image_expmap' function, and start looking at the image making XMM task (

def evselect_image(sources: Union[BaseSource, NullSource, BaseSample], lo_en: Quantity = Quantity(0.5, 'keV'),
) for inspiration in terms of what setup you need to do, and look at my DAXA wrappers for fluximage as well. Try adding some of the initial checks you might see in the XMM image making function, and then we can talk some more about it.

@DavidT3 DavidT3 converted this from a draft issue Nov 20, 2024
@DavidT3 DavidT3 added enhancement New feature or request telescope specific Changes specifically for one telescope supported by XGA, rather than the overall infrastructure labels Nov 20, 2024
@rz-wang rz-wang moved this from Ready to In progress in XGA Chandra Support Nov 21, 2024
rz-wang added a commit that referenced this issue Nov 21, 2024
rz-wang added a commit that referenced this issue Dec 5, 2024
…ot.py" which is should generate Chandra image/expimg/ratemap (basically the CIAO fluximage 'rate' mode). Also, fixed a formatting issue under run.py. (#1264)

Signed-off-by: rz-wang <[email protected]>
rz-wang added a commit that referenced this issue Feb 21, 2025
Signed-off-by: rz-wang <[email protected]>
DavidT3 added a commit that referenced this issue Feb 28, 2025
DavidT3 added a commit that referenced this issue Feb 28, 2025
@DavidT3
Copy link
Owner Author

DavidT3 commented Feb 28, 2025

@rz-wang

Just an FYI, the way you had the chandra_image_expmap function written, specifically this bit:

 # Setting up the top level path for the eventual destination of the products to be generated here
dest_dir = os.path.join(OUTPUT, "chandra", obs_id)

# If something got interrupted and the temp directory still exists, this will remove it
if os.path.exists(dest_dir):
    rmtree(dest_dir)
    os.makedirs(dest_dir)

You would have immediately deleted the entire XGA directory for the current Chandra ObsID - it's useful to be able to be inspired by and take snippets from the product generation methods of other missions, but you need to make sure to read what you've been copying to make sure it all still does what you think it does.

@DavidT3
Copy link
Owner Author

DavidT3 commented Feb 28, 2025

@rz-wang

Oh btw, when writing docstrings for a variable that has a Union type hint, you have to not add the possible types with / in between them, not in a Union. Don't know why but it is a particular foible of Sphinx AutoDoc, which is the package that puts all the docstrings into the documentation (though I have somehow broken a lot of it for the multi-mission version)

So something like this:

:param BaseSource/NullSource/BaseSample sources: A source object, null source, or sample of sources.

The only way I found out about this problem originally was because the docstrings weren't rendering properly

@DavidT3
Copy link
Owner Author

DavidT3 commented Mar 1, 2025

@rz-wang Also, just general advice, when developing code for a module (particularly one you aren't super familiar with yet) you need to try and follow the chain of what will happen when you add new code, to make sure it is going to do what you intended.

For instance, in the chandra_image_expmap function, you add three final output paths for each execution of the command, which of course you were completely right to do as we know it'll make three files, but if you follow the chain of events to the execute_cmd function and look here:

if p_type == "image":

You'll see that it isn't currently designed to handle multiple files of interest being generated at once, unless they all go into the same product (like spectra) - so it needs to be modified.

This isn't a criticism, I know you've never worked on programming like this, just some software engineering advice :)

DavidT3 added a commit that referenced this issue Mar 1, 2025
…ommands) to support the setting up of multiple products (for instance an image, expmap, ratemap from chandra_image_expmap) has begun. For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
…ible execute_cmd errors with Chandra data - yes I know that's confusing but it makes sense to me. For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
…e we haven't yet included a telescope entry in the extra_info dict passed out by chandra_image_expmap. What I don't understand is why it isn't being raised properly by the error callback in ciao_call. For issue #1264 (indirectly)
DavidT3 added a commit that referenced this issue Mar 1, 2025
…eing thrown by the execute_cmd was just a good old fashioned Python error (not from the product) and so looking for the ' is associated source' string to split on and find the source repr was throwing another error! For issue #1264 (indirectly)
DavidT3 added a commit that referenced this issue Mar 1, 2025
…iles loaded into multiple output products. For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
…ifferent products output from one command a little better. For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
DavidT3 added a commit that referenced this issue Mar 1, 2025
DavidT3 added a commit that referenced this issue Mar 1, 2025
…erent types produced by a single function! It should also provide returns that are compatible with the existing states of sas_call and esass_call (so I don't have to change those right now). It can now return a list of products instead of a single product, so will need to adapt ciao_call to make that work. In aid of issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
…already be able to deal with multiple products in the returned 'results' dictionary (containing the set up product instances). It is concerning that I don't remember why that would be, as I am now worried that I might have broken something for another telescope! For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
…, etc. that Ray sets up in the chandra_image_expmap function to the stupidly formatted names that fluximage comes out with. For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
@DavidT3
Copy link
Owner Author

DavidT3 commented Mar 1, 2025

We can now generate images, expmaps, and ratemaps (though they aren't loaded yet)! More importantly I think the infrastructure is properly setup to handle the ways that Chandra software can sometimes produce multiple products in a single function call, with different types.

Now that I've made sure this generation function works, we can make more progress on spectra @rz-wang

Some of the fruits of our labours:

Image

Image

@DavidT3
Copy link
Owner Author

DavidT3 commented Mar 1, 2025

Ah well it now doesn't work for multiple ObsIDs in the same source, so I have broken something else.

Haven't yet tested how it behaves with multiple sources

DavidT3 added a commit that referenced this issue Mar 1, 2025
… it was wiping it each iteration. Hopefully fixing this will make image generation work for sources with multiple Chandra ObsIDs. For issue #1264
DavidT3 added a commit that referenced this issue Mar 1, 2025
…age_expmap up one level of indent, I think they were being set too earlier. For issue #1264
@DavidT3 DavidT3 moved this from In progress to In review in XGA Chandra Support Mar 1, 2025
@DavidT3
Copy link
Owner Author

DavidT3 commented Mar 1, 2025

@rz-wang I'm happy-ish now that the chandra_image_expmap function is doing what I expect, both with single ObsIDs and multiple. The products are also being loaded in to the XGA source storage structure properly.

It isn't writing the image/ratemap/expmap files to the write paths yet (but that's only because I haven't been bothered to put move commands into the end of the command in chandra_image_expmap.

Your immediate task at the beginning of the week should be to figure out (from that notebook, and from how eROSITA is set up) how to convert from the regions that XGA has loaded in, to a region file we can pass to specextract so we can generate a spectrum within a specified aperture with contaminating sources removed.

You could also try and complete the specextract command (now that badpix and aspect files are easily available) - and just see if it runs and loads in the spectrum properly, even if that spectrum isn't useful

DavidT3 added a commit that referenced this issue Mar 3, 2025
…es to the execute_cmd function are raising 'XGADeveloperError('Both the p_type and p_path arguments must be of the same type (both string or both list).' errors - trying to alter the inputs passed in now. This was caused by my work on issue #1264
DavidT3 added a commit that referenced this issue Mar 3, 2025
…s_call decorator can currently pass product paths out. Problem caused by my work on issue #1264
DavidT3 added a commit that referenced this issue Mar 3, 2025
…instruments are stored under the 'combined' instrument (unless the configuration file is structured with an instrument name in front of the attitude file header (in which case diff. instruments might have diff. att. files). Broken by my work on issue #1264
@DavidT3
Copy link
Owner Author

DavidT3 commented Mar 5, 2025

Note for me - need to make sure the output files are named using the XGA convention before I consider this fully working and done.

@DavidT3
Copy link
Owner Author

DavidT3 commented Mar 7, 2025

Alright, realised that fluximage also needs a mask file passed in to deal with any non-standard chip configurations. Will correct that problem in DAXA quickly, but will also need to be dealt with here. I'll move this back to 'in progress' on the project board

@DavidT3 DavidT3 moved this from In review to In progress in XGA Chandra Support Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request telescope specific Changes specifically for one telescope supported by XGA, rather than the overall infrastructure
Projects
Status: In progress
Development

No branches or pull requests

2 participants