DavidT3 / XGA

X-ray: Generate and Analyse is a module designed to make the analysis of XMM observations simple and efficient. It provides an interface with SAS for the creation of XMM data products, as well as a way to easily perform fits (scalable for multiple observations) and retrieve information about an object, all within a Python package.
BSD 3-Clause "New" or "Revised" License
29 stars 3 forks source link

Add support for multiple runs of the same XSPEC model with different fit configurations #1209

Open DavidT3 opened 3 weeks ago

DavidT3 commented 3 weeks ago

This should have been in a long time ago, and I swear I had an issue for this but I cannot find it, but currently the XSPEC fitting functions (and the storage of the results in files, and in XGA objects) have no way of knowing what the fit configuration was. Thus changing the fit configuration doesn't run a different fit, which is terrible.

For instance, say you wanted to fit an absorbed plasma emission model to a cluster, with metallicity frozen and with it not frozen, and compare how the results change - you could not do that right now without setting up two versions of the same source and telling it not to load fits on declaration.

The reason I've never done this is because it is complicated, and more importantly it will add another layer of complexity for the user accessing results, but it does need to be done.

DavidT3 commented 3 weeks ago

The primary things that need doing are:

No doubt there are many more details to work out, BUT HERE WE GO.

DavidT3 commented 3 weeks ago

Will now need to add the same functionality to the Spectrum class - including for the storing of plotting information for the models and XSPEC versions of the spectra - also will change the view method. THEN I'll need to go back to the view methods of AnnularSpectra and change them also.

DavidT3 commented 3 weeks ago

Not doing this yet, but the same process of using fit configuration keys will need to be applied to the fakeit simulations done using spectra - this is in essence issue #111

DavidT3 commented 3 weeks ago

The view methods of AnnularSpectra are all a bit buggered - the view_annulus method has had the conversion process started, but I forgot that they are still very basic compared to the view method of Spectrum, and still can't plot anything if no fit has been done, which is daft. Maybe I'll just completely rewrite them later.

DavidT3 commented 3 weeks ago

Settled on rewriting them later - will add a get_view method to Spectrum (if I haven't already done that, I can't remember), and make use of that for view_annulus.

DavidT3 commented 3 weeks ago

Note-to-self - even though the XSPEC fit functions (or the ones in 'general' anyway) now generate and pass out fit_conf keys, I still need to incorporate that into the script and results file names. This will mean altering the '_write_xspec_script' function.

DavidT3 commented 3 weeks ago

Now I've started down this path, I think the best option for running the cross arf fits would be to control it with an argument to the existing single_temp_apec_profile function, rather than having it in its own function as it is now.

That will necessitate some rewriting, but I just think it makes the most sense from the user point of view. I can also allow them to choose whether they want an initial run with the non-cross-arf implementation first (as always happens as it is currently configured) or whether they just want to jump right in and try to run the first fit with cross-arf.

DavidT3 commented 3 weeks ago

Okay! The storing of multiple fits is working now, and retrieving the different results is working if you have the exact fit conf key. The main next thing to do is write something that the user can pass the parameters they changed to and have it construct the key by adding in the default stuff.

DavidT3 commented 3 weeks ago

Also, I think I'm going to have to allow different sources to have different fit_conf keys when running something like single_temp_apec, because I do allow there to be different start temperatures for a set of sources.

DavidT3 commented 3 weeks ago

I want a way of knowing which arguments to an XSPEC fit function matter in terms of the definition of the fit_conf storage key. Currently those arguments are just defined within each function - this was an okay place to start, but I a) want to be able to access that information outside of the fit functions (i.e. in the fit_conf_from_function function that I'm writing) and b) worry that at some point we'll forget to update the dictionaries that are manually setup in each function.

As such I think I will abstract all those dictionaries specifying what information to include in the fit conf keys to somewhere else, and they will include null entries for arguments to the functions that don't matter, and that way each function can check that no new argument has been added to the function signature that we haven't considered including in the fit conf storage keys

DavidT3 commented 3 weeks ago

I have found myself in a bad circular import situation - this is because of the function I created to help users construct fit_conf keys by only passing the parameters they changed.

What has happened is that now BaseSource imports that function, which in turn imports all the XSPEC fitting functions, which has this huge long knock on effect because they import the BaseSource class, so it is circular at initial import time.

I've been trying the TYPE_CHECKING constant that can be imported from 'typing' which I've only just heard of, but actually I've just thought of a better solution, I can do a local import of the function which is causing the problems in BaseSource.

DavidT3 commented 3 weeks ago

This seems to be sorted out now

DavidT3 commented 2 weeks ago

So I think the reading in of XSPEC fits from disk (i.e. when they have been previously run, and are now stored in the inventory file) should now be essentially fully functional again - the new XSPEC-fit inventory reading method seems to be working well APART from when trying to load in cross-arf based annular spectrum fits, that doesn't seem to be functional again yet,