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 tfCorrection with numPeriodGrouping #106

Open
jusack opened this issue Oct 22, 2024 · 1 comment
Open

Fix tfCorrection with numPeriodGrouping #106

jusack opened this issue Oct 22, 2024 · 1 comment

Comments

@jusack
Copy link
Contributor

jusack commented Oct 22, 2024

Currently it is not possible to use numPeriodGrouping together with tfCorrection (in both getMeasurements and getMeasurementsFD) because the sampled TF data inside the MDF is for a single period.

We have different options:

  1. Just interpolate the TF based on the data inside the MDF and apply it to the grouped periods
  2. Do TF correction on each period before period grouping, this might cause jumps in the signal
  3. MPIMeasurements: Save more samples into the TF if multiple periods are measured -> not according to MDF standard
@nHackel
Copy link
Member

nHackel commented Nov 1, 2024

I think option 1 sounds good. Don't we already have most of that functionality with the TransferFunction struct, sampleTF function and so on?

We could extend the rxTransferFunction call to include the period grouping. I think at the moment that might clash with the setters of MDF. We could change those to rxTransferFunction!, but maybe the dispatch just works out fine with an integer or keyword argument .

Additionally or alternatively, we could also give the tf as a new keyword argument getMeasurement(f, ...; tfCorrection = !isnothing(tf), tf = rxTransferFunction(f, ...), ...) and then a user could supply a TF for the requested period grouping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants