feat: Implementing frequency rebinning for PowerSpectrum and LightCurve#71
feat: Implementing frequency rebinning for PowerSpectrum and LightCurve#71Omiii-215 wants to merge 141 commits intoStingraySoftware:unstablefrom
Conversation
Implement Workflow
Add fourier methods and tests
Implement GTI Reading and Handling
…nmath Bump NanMath requirements
Merge GTI modifications into main
It seems entirely overkill to run on 3 different operating systems and for two different Julia versions for a package that is still in heavy development. Currently we just need the CI to be rapid and automate code checking. There is nothing in this package that is specific to a given OS, and we can worry about compat with Julia versions once we approach a first stable version.
Fix StingraySoftware#16 & StingraySoftware#17 Removed Metadata.jl dependency
…ation and adding test includes
- Restore GTI filtering in - Harden - Rename to - Update docstrings and plotting recipes - Verify rebinning logic with tests
There was a problem hiding this comment.
Hey @Omiii-215, thanks for ur sub PR you have really done a great work {u can once review my comments. specially this one try testing via some datas} :)
| [deps] | ||
| Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4" | ||
| Stingray = "2045e982-6adb-4e09-8364-0d2cc8a3fe4f" | ||
|
|
There was a problem hiding this comment.
While running the project, we can't include Stingray in the project.toml {this is the document config}
So I know what happened here, you may tried to make docs you can just use
using Stingray
Stingray.someFuncunction()avoid this scope
| for (band_name, events) in events_dict | ||
| ps = AveragedPowerspectrum(events, segment_size; norm = norm) | ||
| ps = AveragedPowerSpectrum(events, segment_size; norm = norm) | ||
| power_vec = freq_mult ? ps.power .* ps.freq : ps.power |
There was a problem hiding this comment.
Thanks for noticing this :)
| counts::Vector{Int}, | ||
| method::Symbol; | ||
| gaussian_errors::Union{Nothing,Vector{<:Real}} = nothing, | ||
| gaussian_errors::Union{Nothing,Vector{<:Real}} = nothing |
| # Throws | ||
| - `ArgumentError`: If custom errors length doesn't match bin count | ||
| """ | ||
| function set_errors!(lc::LightCurve{T}) where {T} |
There was a problem hiding this comment.
you revert to
function set_errors!(lc::LightCurve{T}) where {T}since"Curly braces become necessary when you have multiple type parameters." Otherwise, it is the same
| - [`set_errors!`](@ref): Set error method and calculate errors | ||
| """ | ||
| function calculate_errors!(lc::LightCurve{T}) where {T} | ||
| function calculate_errors!(lc::LightCurve{T}) where T |
| merge!(extra_metadata, eventlist.meta.extra) | ||
| if !isnothing(eventlist.meta.gti) | ||
| extra_metadata["gti"] = eventlist.meta.gti | ||
| end |
There was a problem hiding this comment.
This may hinder review it once since merging the gtis is crucial
| throw(ArgumentError("No events remain after GTI filtering")) | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
i didnt get what the use of this?
| new_powers, | ||
| new_errs, | ||
| ps.norm, | ||
| 0.0, # df is variable |
| using Test | ||
| using Stingray | ||
| using Statistics | ||
|
|
There was a problem hiding this comment.
u can include these files in the runtest.jl {it is already there}
| ps.metadata | ||
| ) | ||
| end | ||
| end |
There was a problem hiding this comment.
You need to test these new functions via data present here
Use clean data "ni1200120104_0mpu7_cl.evt" from "ni1200120104_0mpu7_cl.evt.gz" so u can run the test easily via setting up a Jupyter notebook and include powespectrum and plotting.jl and lightcurve and event.jl files in ur dirs and start testing, use your function via first using readevents function u may refer to:
and start making Plots : )
- LightCurve: Restore apply_filters, safe metadata merge, correct GTI logic - PowerSpectrum: Fix logrebin df calculation to avoid 0.0 - Docs: Fix Pkg.develop pattern and deps in docs/make.jl - Tests: Add robust invariant checks to test_rebinning.jl
|
@kashish2210 Verification with Real Data: I have verified these changes locally using a clean event file ( LightCurve Rebinning: Validated that total counts are preserved exactly (
|
This looks good to me nice work @Omiii-215 |
|
Just one more thing we should not focus on docs since, for this scope it is not needed so u can revert changes in that |
Done |
|
@matteobachetti and @fjebaker, we can start testing this branch, waiting for your review |
|
@Omiii-215 in the current state, the PR is full of formatting changes that really make review difficult. Also, it seems that you removed at least one function. Please clean up the PR, only maintain the actual important changes. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #71 +/- ##
===========================================
Coverage ? 66.97%
===========================================
Files ? 12
Lines ? 3770
Branches ? 0
===========================================
Hits ? 2525
Misses ? 1245
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@matteobachetti I've completely cleaned up the PR, reverted all unrelated formatting/naming changes (like PowerSpectrum) and restored the accidentally removed functions. The branch now only cleanly introduces the new rebin and logrebin implementations, and all tests are passing. :) |





Summary
Adds frequency rebinning support for power spectra.
What’s included
rebin(ps, factor)logrebin(ps, f)rebinoverload forLightCurveNotes