Improved TDR with Inverse Chirp Z-transform#358
Conversation
inverse chirp-z transform added
|
|
||
| Mode mode; | ||
| WindowFunction window; | ||
| double velocityFactor; |
There was a problem hiding this comment.
each trace already has a velocity factor setting. See
andThe ICZT will always be attached to a trace and should probably grab the velocity factor from that instead of having its own setting.
| ); | ||
|
|
||
| // Helper functions | ||
| std::vector<double> makeLowpassWindow(size_t n); |
There was a problem hiding this comment.
can avoid code duplication by using the functions from https://github.com/jankae/LibreVNA/blob/master/Software/PC_Application/LibreVNA-GUI/Traces/Math/windowfunction.h instead of implementing the window equations here again
| return df_median; | ||
| } | ||
|
|
||
| complex<double> ICZT::estimateDC(const vector<complex<double>>& s11) |
There was a problem hiding this comment.
This function implements a very simple DC approximation by just taking the first sample. This will not work very well for almost all sweeps because the first sample will be quite a bit away from DC.
Compare with
LibreVNA/Software/PC_Application/LibreVNA-GUI/Traces/Math/tdr.cpp
Lines 315 to 318 in 5dafe7a
for a slightly better implementation. But even that is not very good.
Suggestion: Provide a better DC extrapolation method in a dedicated file (or in the Trace class) and use that for both the TDR and ICZT)
| } | ||
|
|
||
| // Rate limiting | ||
| this_thread::sleep_until(lastCalc + duration<double>(0.05)); |
There was a problem hiding this comment.
Rate limiting for the TDR is configurable in the preferences. You could use the same setting here instead of making it a fixed value
| vector<complex<double>> y(t_s.size()); | ||
| const size_t block_size = 256; | ||
|
|
||
| for(size_t start = 0; start < t_s.size(); start += block_size) { |
There was a problem hiding this comment.
I have not taken a close look at the math yet but do have a couple of questions (might be dumb questions):
- why do you have the outer loop with the block size? Unless I am missing something, this is not really any different than processing all samples in one loop. I.e. right now you are computing samples 0..255 followed by 256..511 and so on. But for each block of samples you just iterate through every sample? Couldn't you just loop over all samples from 0..end and skip the block processing? If I am right, the block processing does nothing to improve performance but complicates the code a bit
- This seems to be the naive approach of calculating the ICZT with a complexity of O(n^2). Is this really still performant enough for typical sweep settings? I thought about implementing an ICZT a long time ago myself but never ended up doing so because I could not find a good implementation with better complexity (such as O(n*log n)
|
Hi and thank you very much for this PR. Having an ICZT would indeed be very useful. I have added a few comments about things I believe can be improved (mostly just avoiding duplicated code and using already available settings instead of implementing them yourself).
Sure, please point out anything that does not work as you think it should and I will see how I can help on the Qt side. |
|
Looks like there are some build problems. Did you manage to compile this on your side? At first glance this might just be the iczt.cpp file not being build because it is not included in the LibreVNA-GUI.pro file. Maybe you forgot to add that file to the commit? |
2 participants
Added inverse chirp-z transform with the features:
With the lowpass step you can connect a bunch of coax cables and attenuators, and see the impedance change in every one of the connections.
Observe that this math break down hard if the sweep is not frequency linear!
I had some issues with the GUI and QT, so not all features are as it should. That is far from what I know, and dealing with QT is probably better left to someone knowing what they are doing.