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

Plot unfolded data and model #111

Merged
merged 7 commits into from
Jun 18, 2024
Merged

Plot unfolded data and model #111

merged 7 commits into from
Jun 18, 2024

Conversation

phajy
Copy link
Contributor

@phajy phajy commented Jun 14, 2024

This is a draft PR for plotting unfolded data and model (see #82). This draft will use the XSPEC method of calculation. It is a WIP and not ready to be merged. There are a couple of things I don't know how to do, and perhaps a bug I need to chase down.

@phajy
Copy link
Contributor Author

phajy commented Jun 15, 2024

This now produces unfolded E F_E plots that match those in XSPEC (produced using XSPEC's plot eeufs command). The key change that was required in the last commit was to divide by $\Delta x$ to get the fluxes in "per keV" units.

  • The code needs to be tidied up.
  • It would be awesome for units to propagate through everything then the axes could be automatically labelled.
  • There are some data.data.'s that ultimately need cleaning up.
  • The "power" should be a parameter (I think it is now) so it is easy to plot unfolded, F_E, and E F_E plots where we multiple by 1, E, and E squared, respectively.
  • Allow the legend to be turned on/off.
  • Remove the diagnostic print statements.
  • Do we want the model to be stepped like in XSPEC?

Here are two example plots. The first from XSPEC the second from SpectralFitting showing good agreement.

Screenshot 2024-06-15 at 21 26 44 Screenshot 2024-06-15 at 21 27 19

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 45 lines in your changes missing coverage. Please review.

Project coverage is 63.22%. Comparing base (6ab9ac7) to head (7a19436).
Report is 21 commits behind head on main.

Files Patch % Lines
src/plots-recipes.jl 0.00% 45 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   66.24%   63.22%   -3.03%     
==========================================
  Files          41       41              
  Lines        2216     2344     +128     
==========================================
+ Hits         1468     1482      +14     
- Misses        748      862     +114     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

phajy and others added 2 commits June 17, 2024 10:48
Make `invokemodel` with dataset as the domain argument be a way of
evaluating the model on the same domain as the data without necessarily
folding the data.
@fjebaker fjebaker marked this pull request as ready for review June 17, 2024 15:49
@fjebaker
Copy link
Owner

@phajy I've added two commits and marked ready for review, if that's okay?

@fjebaker
Copy link
Owner

See also #113

@phajy
Copy link
Contributor Author

phajy commented Jun 18, 2024

Thanks @fjebaker. This looks great and I am happy for you to merge is with the main branch.

@fjebaker fjebaker merged commit ba912b8 into fjebaker:main Jun 18, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants