-
Notifications
You must be signed in to change notification settings - Fork 356
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
allow psd var sample rate to change as needed #4541
allow psd var sample rate to change as needed #4541
Conversation
pycbc/psd/variation.py
Outdated
|
||
# Resample the data | ||
strain = resample_to_delta_t(strain, 1.0 / 2048) | ||
srate = int(strain.sample_rate) | ||
strain = resample_to_delta_t(strain, 1. / srate) |
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.
Can I check: wouldn't strain
here already be at the opt.sample_rate
? What sample rate would it have if not opt.sample_rate
? Do I not achieve the same thing by just removing the resample_to_delta_t
line?
@Moz-Port has left academia now, but I'm not aware of any reason why this was resampled specifically to 2048Hz ... I hope there's not a particular reason why this needs doing that we're missing ???
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.
Oh yes! We don't need to resample it again!
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.
I'm a bit rusty but I don't think removing the resampling will cause any problem.
When I first wrote this I was often using this code as a script to analyze raw data, so it must be a relic from my initial project.
…a opts) so it is not even needed. Does PSD var really need fixed sample rate of 2048?
* Use same sample rate as strain for PSD variation instead of hardcoded 2048 Hz * make srate int * strain is already preprocessed (resampled to requested sample rate via opts) so it is not even needed. Does PSD var really need fixed sample rate of 2048? * Removing unused resample to make codeclimate happy in variation.py --------- Co-authored-by: Bhooshan Gadre <[email protected]>
* Use same sample rate as strain for PSD variation instead of hardcoded 2048 Hz * make srate int * strain is already preprocessed (resampled to requested sample rate via opts) so it is not even needed. Does PSD var really need fixed sample rate of 2048? * Removing unused resample to make codeclimate happy in variation.py --------- Co-authored-by: Bhooshan Gadre <[email protected]>
* Use same sample rate as strain for PSD variation instead of hardcoded 2048 Hz * make srate int * strain is already preprocessed (resampled to requested sample rate via opts) so it is not even needed. Does PSD var really need fixed sample rate of 2048? * Removing unused resample to make codeclimate happy in variation.py --------- Co-authored-by: Bhooshan Gadre <[email protected]>
* Use same sample rate as strain for PSD variation instead of hardcoded 2048 Hz * make srate int * strain is already preprocessed (resampled to requested sample rate via opts) so it is not even needed. Does PSD var really need fixed sample rate of 2048? * Removing unused resample to make codeclimate happy in variation.py --------- Co-authored-by: Bhooshan Gadre <[email protected]>
* Use same sample rate as strain for PSD variation instead of hardcoded 2048 Hz * make srate int * strain is already preprocessed (resampled to requested sample rate via opts) so it is not even needed. Does PSD var really need fixed sample rate of 2048? * Removing unused resample to make codeclimate happy in variation.py --------- Co-authored-by: Bhooshan Gadre <[email protected]>
sample rate of 2028 was hardcoded in the PSD var calculation. This was an issue if we wanted to use a different sample rate!