-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fill pulse traces with NaN/-1 if data is too short #245
base: master
Are you sure you want to change the base?
Conversation
…rain trace is too short
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #245 +/- ##
=======================================
Coverage 73.87% 73.87%
=======================================
Files 23 23
Lines 2989 2989
=======================================
Hits 2208 2208
Misses 781 781 ☔ View full report in Codecov by Sentry. |
87fd49e
to
8c63960
Compare
# The end of this pulse is beyond the trace length, try to | ||
# copy what remains and fill with placeholder as needed. | ||
|
||
if start < trace_len: | ||
# There is still some data to copy into this pulse. | ||
memcpy(&out[i, 0], &traces[j, start], | ||
(trace_len - start) * sizeof(traces[0, 0])) | ||
|
||
# Fill in the rest of this pulse with the placeholder value. | ||
for k in range(max(trace_len - start, 0), samples_per_pulse): | ||
out[i, k] = placeholder_val | ||
else: | ||
memcpy(&out[i, 0], &traces[j, start], bytes_per_pulse) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks it possible to make a bit simple:
# The end of this pulse is beyond the trace length, try to | |
# copy what remains and fill with placeholder as needed. | |
if start < trace_len: | |
# There is still some data to copy into this pulse. | |
memcpy(&out[i, 0], &traces[j, start], | |
(trace_len - start) * sizeof(traces[0, 0])) | |
# Fill in the rest of this pulse with the placeholder value. | |
for k in range(max(trace_len - start, 0), samples_per_pulse): | |
out[i, k] = placeholder_val | |
else: | |
memcpy(&out[i, 0], &traces[j, start], bytes_per_pulse) | |
# The end of this pulse is beyond the trace length, try to | |
# copy what remains and fill with placeholder as needed. | |
samples_to_copy = min(max(trace_len - start, 0), samples_per_pulse) | |
# copy if data available | |
memcpy(&out[i, 0], &traces[j, start], samples_to_copy * sizeof(traces[0, 0])) | |
# Fill in the rest of this pulse with the placeholder value. | |
for k in range(samples_to_copy, samples_per_pulse): | |
out[i, k] = placeholder_val |
end = start + samples_per_pulse | ||
|
||
if end > trace_len: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end = start + samples_per_pulse | |
if end > trace_len: |
When using the pulse separation feature in the
AdqRawChannel
component, currently an exception is thrown if the underlying train trace is too short for all pulses. This is not very useful however, as there's little way for users to fix that in any way, e.g. only get data for those pulses that fit, or reduce the pattern asked for.Rather than throwing an exception, this PR instead fills the missing samples with
NaN
(for floating types, most likely) or-1
(in case of integer types, only with non-default settings). This maintains the information which data is missing, and the returned array can easily be truncated if desired.