search

PAHFIT / pahfit

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

Simple FWHM Fix #177

Closed jdtsmith closed 2 years ago

jdtsmith commented 2 years ago

This is a simple fix for the overly generous 2x FWHM (see #174), which led to spurious fits. This simply hard-codes the 10% for now. In the near term, we plan to implement a new Instrument Pack/Science Pack with generator, making this code irrelevant.

karllark commented 2 years ago

Also needs the testing parameters update as this changes the results for M101.

jdtsmith commented 2 years ago

I'm not as familiar with the test structure; can someone else take a look at that?

karllark commented 2 years ago

For now, it might be best to accept failing tests and merge if we have good confidence that it is just needing to update the cached fit values. There are now multiple PRs that have this test failure. Then I can do a "quick" PR to fix the failing tests after the different merges are done.

Or another solution is that I could provide the needed code snippet that would generate the lines that would need to be cut/pasted in to the test function.

Thoughts?

jdtsmith commented 2 years ago

If we have a few ongoing changes in this category, perhaps “clean up after” is the right approach. I demoed my own branch at the workshop yesterday so probably good to get some of these in.

karllark commented 2 years ago

Merging now. Will fix failing tests in a separate PR.