Add single_freq parameter, bug fixes #164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carlosgjs
requested review from
mdenolle,
niyiyu and
chengxinjiang
as code owners
niyiyu
approved these changes
Aug 1, 2023
| ) -> List[Tuple[Channel, ChannelData]]: | ||
| ch_data_refs = [executor.submit(store.read_data, ts, ch) for ch in channels] | ||
| ch_data = _get_results(ch_data_refs) | ||
| tuples = list(zip(channels, ch_data)) | ||
| return _filter_channel_data(tuples, samp_freq) | ||
| return _filter_channel_data(tuples, samp_freq, single_freq) |
There was a problem hiding this comment.
Thanks Carlos,
If I understand this this function right, it returns the channels at the frequency closest to samp_freq when single_freq = True. While single_freq = False, it returns all channels with frequency greater than samp_freq. By default, the fft_param set this switch as True.
Is there any test needs to be added accordingly?
There was a problem hiding this comment.
ha I see the tests below now. Looks great!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some datasets (e.g. SCEDC S3 bucket) the data comes in multiple sampling rates and we need a way of selecting one. So the desired sampling rate is passed as a config parameter and we find the sampling rate available in the data that is the closest (but equal or greater) than the config. Then we use only the data at that exact frequency.
However, in other scenarios we want to use all the data, as long as its above the desired sampling rate. This PR adds a configuration parameter to control this.
It also adds a couple of checks to avoid crashing when there's missing data.