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

Fix signalstats implementation when having samples with units #53

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

fhagemann
Copy link
Contributor

@fhagemann fhagemann commented Jan 31, 2025

In PR #41, code was introduced that fails if the samples have units:

using RadiationDetectorDSP
using RadiationDetectorSignals
using Unitful

wf = RDWaveform((0:100)u"ns", randn(101)u"C")
signalstats(wf, 0u"ns", 100u"ns")
ERROR: DimensionError: 0.0 C and 1.0373625148197236 C^2 are not dimensionally compatible.
Stacktrace:
 [1] _isless
   @ ~/Software/Unitful.jl/src/quantities.jl:250 [inlined]
 [2] isless(x::Quantity{Float64, 𝐈 𝐓, Unitful.FreeUnits{…}}, y::Quantity{Float64, 𝐈^2 𝐓^2, Unitful.FreeUnits{…}})
   @ Unitful ~/Software/Unitful.jl/src/quantities.jl:240
 [3] max(x::Quantity{Float64, 𝐈^2 𝐓^2, Unitful.FreeUnits{…}}, y::Quantity{Float64, 𝐈 𝐓, Unitful.FreeUnits{…}})
   @ Base ./operators.jl:481
 [4] _signalstats_impl(X::StepRange{Quantity{…}, Quantity{…}}, Y::Vector{Quantity{…}}, idxs::UnitRange{Int64})
   @ RadiationDetectorDSP ~/.julia/packages/RadiationDetectorDSP/zR8oQ/src/signalstats.jl:54
 [5] signalstats(input::RDWaveform{…}, start::Quantity{…}, stop::Quantity{…})
   @ RadiationDetectorDSP ~/.julia/packages/RadiationDetectorDSP/zR8oQ/src/signalstats.jl:21
 [6] top-level scope
   @ REPL[8]:1

After this PR, signalstats does not fail and the output looks like this:

(mean = 0.07191460017009416 C, sigma = 0.9393140224212858 C, slope = -0.0040645798613581545 C ns^-1, offset = 0.27514359323800186 C)

@oschulz oschulz merged commit c60aaf9 into JuliaPhysics:main Jan 31, 2025
8 of 9 checks passed
@oschulz
Copy link
Member

oschulz commented Jan 31, 2025

Thanks!

JuliaRegistries/General#124107

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.

2 participants