Skip to content

Conversation

@HarrisonS-Phys
Copy link
Collaborator

…e, and removed extraneous lines of code

@farr
Copy link
Owner

farr commented Nov 14, 2024

I'm curious about the return values here---why don't you return the time-domain line model instead of the real and imag freq components? (Just thinking about Fourier transform conventions---everything input to the cleaner is real, and if everything output from the cleaner is also real, then we don't have to fuss about definitions of FTs, because it's all internal; as well, it's easier to understand the time-domain line model, no?)

Also: why are you just returning the real part of the data frequency series? Sort of the same question?

@farr
Copy link
Owner

farr commented Nov 14, 2024

Also, also: isn't there an extra s in the return pred_sampless? Does this code path even work? :cringe:

@farr
Copy link
Owner

farr commented Nov 14, 2024

Finally, finally (I hope): note that you can extract the frequency components of the line model already from the predicted samples---so maybe it would be better just to store the td line model in the prediction instead of returning separately?

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