Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fitting - minor bugs and possible tweaks #74

Open
ianan opened this issue Jun 30, 2022 · 1 comment
Open

Fitting - minor bugs and possible tweaks #74

ianan opened this issue Jun 30, 2022 · 1 comment

Comments

@ianan
Copy link

ianan commented Jun 30, 2022

Provide a general description of the issue or problem.

Testing the fitting for a variety of NuSTAR sources and finding similar results to XSPEC . But also finding a number of inconsistencies, minor bugs and things that need tweaked:

  • For default plotting y-axis label is wrong - not a "Count Spectrum [cts/s/keV]" as units count/s/keV. Just change label to units, i.e. count/s/keV?
  • For default plotting, add option to turn off annotate plot with fit results, i.e. _plot_params in https://github.com/sunpy/sunxspex/blob/818ae4f7f47db1c3049f02a57a2aaf8ea2eac963/sunxspex/sunxspex_fitting/fitter.py#L3005
  • Can easily extract spectrum and fits for own manual plotting but:
    • resid=spec.plotting_info['spectrum1']['residuals'] doesn't return the residuals, but a modified version of them for the internal plotting - every element repeated so need to do spec.plotting_info['spectrum1']['residuals'][::2].
    • Naming of things a bit inconsistent particularly in terms of plurals, i.e. counts, count_rates, count_channel_error, count_rate_errors.
    • Also have count_rate in loaded_spec_data but count_rates in plotting_info.
    • What is called count_rates (and count_rate) is actually count/s/keV - probably call it flux though flux usually count/s/keV/cm2?
    • It would help if each had a descriptor saving what they are, or include actual units?
  • Not clear the units/scaling of the fit parameters. So add descriptor or units when returning something like spec.params["T1_spectrum1"]?
    • Also confusing fit uses different thermal params from actual model thermal.thermal_emission, i.e. MK and 1e26 cm^-3 against keV and cm^-3. Is this something to do with fit optimisation?
  • Need a simpler (and more) examples, just doing basic fit but also show how to extract fit params and other stuff.
@settwi
Copy link
Contributor

settwi commented Mar 6, 2024

i think these are really good points--refactor should make things clearer.
keeping units attached to the model parameters might be the best way to do that but also might slow things down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants