PAHFIT / pahfit

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

Fix issue 150 #167

Closed ThomasSYLai closed 2 years ago

ThomasSYLai commented 2 years ago

Fix #150. Now PAHFIT can fit spectrum with its shortest wavelength greater than 5.5 µm

drvdputt commented 2 years ago

I have also run into this issue, and I think I made a temporary fix at some point. This does make me wonder: why does the 5.0 - 5.5 micron range need a different behavior than wav < 5.0 or wav > 5.5?

ThomasSYLai commented 2 years ago

@drvdputt There is nothing special. The 5.5 µm was set in the original IDL PAHFIT. When developing the enhanced PAHFIT to fit akari+spitzer, I made it start at shorter wavelengths depending on the shortest available wavelength in akari (~2.6 µm). At some point we can probably just use min(obs_x) + 0.1 as the reference point. I assume the fitting of the continuum does not care about this reference wavelength that much as long as it's near the trough where the stellar component meets dust components.

karllark commented 2 years ago

@BethanyRS : Please review this PR. Thanks.

BethanyRS commented 2 years ago

I'm still having a bit of trouble getting PAHFIT to run on a cut spectrum. When I use a spectrum that ranges from 8-14 microns, I get the error "ValueError: max() arg is an empty sequence." According to the error message, this occurs in line 77 of "component_models":

kvt_int_short = kvt_int_short_tmp * (kvt_int[0] / max(kvt_int_short_tmp))

Does this error have something to do with

kvt_wav_short = in_x[in_x < min(kvt_wav)]

(line 73 of "component_models")? It looks like kvt_wav starts at 8 microns.

karllark commented 2 years ago

@BethanyRS is this with Thomas' branch of pahfit? This branch has not been merged yet, hence you would have to get his branch to test if this fix works.

BethanyRS commented 2 years ago

Unless I'm misunderstanding how switching branches works, my comment should be for the branch of this PR

karllark commented 2 years ago

Unless I'm misunderstanding how switching branches works, my comment should be for the branch of this PR

Great thanks for confirming. @ThomasSYLai can you comment on why @BethanyRS's test is not successful on this branch?

jdtsmith commented 2 years ago

Looks like our parametric dust attenuation curve is brittle if you don't have shorter wavelength coverage. This should be improved.

jdtsmith commented 2 years ago

I think it's fine to merge this simple fix for now. Longer term as we improve the "guessing" algorithm for parameter starting positions, we can simplify this somewhat. It will also need to be adapted to operate directly on the "Features" table, which will simplify some of the logic since we have the kind column now. I.e. just select the starlight_continuum parameter kind directly.

BethanyRS commented 2 years ago

Just to touch base on this again, I'm still getting the same error when I cut my spectrum.

line 77, in kvt
    kvt_int_short = kvt_int_short_tmp * (kvt_int[0] / max(kvt_int_short_tmp))
ValueError: max() arg is an empty sequence

For reference, this time I took Orion_H2S1_ISO-SWS_merged in pahfit/pahfit/data/ISO_SWS and cut the data points so that they were between 8-14 microns. I tried to post the altered Orion_H2S1_ISO-SWS_merged here, but the file type wasn't supported. However, unless I'm misunderstanding something, I think that the same error will occur for any set of data that starts after 8 microns.