PAHFIT / pahfit

Model Decomposition for Near- to Mid-Infrared Spectroscopy of Astronomical Sources
https://pahfit.readthedocs.io/
18 stars 26 forks source link

refactor all model-framework-specific code #250

Closed jdtsmith closed 4 months ago

jdtsmith commented 1 year ago

Once the dev branch is merged, I'd like to work on factoring out all model-framework dependencies (e.g. astropy.model specifics). This will "make room" for the (astropy-less) new model to be substituted in or out, and to keep the astropy version too. In this way we can do testing/comparison and have a backup if something fails.

Things I think will need to happen:

All the other "generic" model information, features setup and manipulation, etc., can stay in model.py. Hoping plot, guess, fit and others will then become "pure methods", with the help of the ABC.

Thoughts?

drvdputt commented 1 year ago

I've thought about this before to solve a minor issue. It always annoyed me that the different components came from different places, e.g. this in the old base.py implementation

from astropy.modeling.functional_models import Gaussian1D
from pahfit.component_models import (
    BlackBody1D,
    ModifiedBlackBody1D,
    S07_attenuation,
    att_Drude1D,
)
from astropy.modeling.physical_models import Drude1D

Giving all of these an identical interface will make things easier, especially because we can iron things out in terms of the parameter definitions, like power vs amplitude, and fwhm (used by drude) vs stddev (used by gaussian). Would be nice if the Model code didn't have to convert these back and forth (as it's doing now for fwhm).

drvdputt commented 1 year ago

And the Model code should ideally be framework agnostic.

As an example of what kind of functionality we would need: Right now, the function parse_astropy_result relies on the fact that you can iterate over the components of an astropy model. With this refactor, it could iterate over a list of component info returned by an abstract function, that has consistent parameter names for each feature (ideally, those names are identical to the column names of the features table). That way the backend can be switched out.

jdtsmith commented 1 year ago

Exactly! All the ugly interfacing to astropy.modeling or scipy.optimize gets hidden behind an abstract class with a guaranteed API. Thinking maybe ModelCore is a good name. But even parse_astropy_result would become a ModelCore platform-specific thing. We'll have to noodle a bit with the minimal API to express this all. I would love to target that API for new_model without the conversion step. Then using new_model or old_model becomes the flip of a property and/or constructor keyword.

For speed (for me anyway), I will probably also want a bit of factoring on the other (more bare-bones) side. Most import case is for plotting: a _plot_raw (or so) function which has very simple inputs: wavelengths and a set of "labeled" Y-vectors. This is for quick reproduction without all the feature unpacking, handing to the component model methods (Gaussian, etc.). This would be used as a "shortcut" for "during the fit" plotting, where you will already have accumulated such things. Of course the real Model.plot will be a "pure" function that consults the feature table, and requests from the appropriate ModelCore interface the (platform-agnostic) Gaussian, etc. to build a richer (if slower) plot from the features themselves.

jdtsmith commented 4 months ago

I think these thoughts have been well-captured in our newer Fitter API discussions. TBD is whether Fitter contains method endpoints for model components like Gaussian, or just the evaluate_model method.