PAHFIT / pahfit

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

Add fitting test (and refactor scripts) #113

Closed karllark closed 3 years ago

karllark commented 3 years ago

Add in a fitting test. Simply reads in the spitzer scipack and M101 nucleus spectrum, runs the fit, and then compares the fit parameters to a cached array of the parameters. Will fail if the parameters are not close (machine precision I think).

Refactored run_pahfit and plot_pahfit scripts to have common functions to avoid code duplication and allow for easier testing. This refactoring will also make it easier to support other fitters, units, etc. in the future.

codecov-commenter commented 3 years ago

Codecov Report

Merging #113 (6df4037) into master (e7d7f2a) will increase coverage by 21.08%. The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #113       +/-   ##
===========================================
+ Coverage   54.71%   75.80%   +21.08%     
===========================================
  Files           4        5        +1     
  Lines         265      310       +45     
===========================================
+ Hits          145      235       +90     
+ Misses        120       75       -45     
Impacted Files Coverage Δ
pahfit/base.py 60.89% <0.00%> (+16.96%) :arrow_up:
pahfit/helpers.py 92.30% <92.30%> (ø)
pahfit/component_models.py 100.00% <0.00%> (+56.75%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e7d7f2a...6df4037. Read the comment docs.

karllark commented 3 years ago

One question: for 'test_fitting_spitzer.py', you use maxiter=200 (ln 17) while the default is set to 1000. Is this to decrease the time for testing or is this a leftover from the previous setting?

For testing so it is fully reproducible regardless of the default maxiter setting. And to reduce the time needed.

karllark commented 3 years ago

And how do I show that part of the code in this comment (like Alexandros did)?

If you go to the "Files changed" tab, you can highlight specific lines and add a comment/review. Use your mouse and the "+" symbols.

els1 commented 3 years ago

And how do I show that part of the code in this comment (like Alexandros did)?

If you go to the "Files changed" tab, you can highlight specific lines and add a comment/review. Use your mouse and the "+" symbols.

Thanks.

karllark commented 3 years ago

Two reviews enough? I think it is ready to merge.

els1 commented 3 years ago

Two reviews enough? I think it is ready to merge.

Fine with me. But there is one check which was unsuccessful ...

karllark commented 3 years ago

But there is one check which was unsuccessful ...

This is codacy being over careful in the default value for a variable in a function definition. It can be ignored.