Skip to content

Allow mixed precision input for compression - #5397

Open
ahnitz wants to merge 3 commits into
gwastro:masterfrom
ahnitz:pr-compress-fixes
Open

Allow mixed precision input for compression#5397
ahnitz wants to merge 3 commits into
gwastro:masterfrom
ahnitz:pr-compress-fixes

Conversation

@ahnitz

@ahnitz ahnitz commented Aug 6, 2026

Copy link
Copy Markdown
Member

This is a minor feature change that does type promotion rather than raising an error if mixed precisions are given for the compression code.

The main reason for this to make it easier to use the pycbc_bank_fir code, introduced in #5395 with precalculated compressed waveforms internally. These can be stored at inconsistent resolutions with the internal testing. It's easier to add here rather than add a few places where we do the typo promotion elsewhere.

ahnitz added 3 commits August 5, 2026 19:46
Commit 7f183b8 ("allow no psd in waveform compression") dropped the
psd[:len(htilde)] slice when adding the psd-is-None branch. htilde is
truncated earlier in this function to kmax = min(len(htilde),
len(hdecomp)), which can be shorter than the original psd whenever
sample_points/fmax covers less than the full waveform band (compressing
over a subset of the frequency range, a supported use of this function).
Without the slice, that combination crashes with a length-mismatch
ValueError; reproduced directly by compressing a TaylorF2 waveform over
half its band with a full-band PSD.
@ahnitz
ahnitz marked this pull request as ready for review August 6, 2026 00:21

@GarethCabournDavies GarethCabournDavies left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sensible I think.

It does force a real datatype - i.e if the out.precision is complex (so not in _real_dtypes), I think this will break. The if out is None branch above will create a complex type out (I can't see what happens with amp and phase there actually), so its a little confusing

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