Version: (using KDE KDE 3.5.0) Installed from: Compiled From Sources OS: Linux Hi all- I'd like to remove the KstPSDGenerator and have KstCSD use KstPSD instead. There's almost complete code duplication right now. To do this, I'll probably move from QValueVectors to something like KstVector for holding the CSD's temporary power spectrum. There may be a slight performance hit, but I think It should be manageable. Does this sound good? Duncan.
KstCSD should not use KstPSD. It breaks the object model. Furthermore please do not introduce new performance penalties. We spend a huge amount of time profiling Kst and we don't want to lose that effort. The use of QValueVector is already a performance loss compared with what we had before. I'd rather see KstPSDGenerator fixed and have both KstPSD and KstCSD use it.
Hi George, Ok then- how about I make a utility class called KstPSDCalculator which will be able to calculate a PSD for an array of doubles, and adapt both KstPSD and KstCSD to use it. Then we can remove KstPSDGenerator. There should be a performance increase in this case, because we'll also be getting rid of the QValueVectors. In the meantime, can you please explain the 'object model' which is broken? To my ear, a CSD is nothing but a collection of PSDs, and so it makes perfect sense to have a CSD using them.
> Ok then- how about I make a utility class called KstPSDCalculator which > will be able to calculate a PSD for an array of doubles, and adapt both > KstPSD and KstCSD to use it. Then we can remove KstPSDGenerator. There > should be a performance increase in this case, because we'll also be > getting rid of the QValueVectors. Ok sounds fine to me. > In the meantime, can you please explain the 'object model' which is broken? > To my ear, a CSD is nothing but a collection of PSDs, and so it makes > perfect sense to have a CSD using them. A 'PSD' and 'KstPSD' are distinctly different things. KstPSD is a dataobject implementation that represents specifically one PSD. The data objects are designed to be chained via inputs and outputs of primitives only. If something else needs the PSD algorithms, it should do so via a shared PSD implementation, not the KstPSD object itself.
SVN commit 553760 by dhanson: CCBUG:129168 copy KstPSDGenerator code into PSDCalculator utility class. comment some of the numerical factors so we know where they come from. next step will be converting KstCSD to use this class, and then KstPSD. A psdcalculator.cpp [License: GPL (v2+)] A psdcalculator.h [License: GPL (v2+)]
SVN commit 553967 by dhanson: CCBUG:129168 move KstCSD over to PSDCalculator. M +80 -96 kstcsd.cpp M +11 -6 kstcsd.h
SVN commit 553968 by dhanson: CCBUG:129168 add PSDCalculator to Makefile.am so that KstCSD will compile. M +1 -0 Makefile.am --- trunk/extragear/graphics/kst/src/libkstmath/Makefile.am #553967:553968 @@ -37,6 +37,7 @@ dialoglauncher.cpp \ eparse-eh.cpp \ eparse.c \ + psdcalculator.cpp \ escan.c libkstmath_la_LDFLAGS = -version-info @KST_LIBKST_VERSION@ $(all_libraries) -no-undefined
SVN commit 553979 by dhanson: CCBUG:129168 let PSDCalculator interpolate holes. M +20 -4 psdcalculator.cpp --- trunk/extragear/graphics/kst/src/libkstmath/psdcalculator.cpp #553978:553979 @@ -26,6 +26,7 @@ #include "kstdebug.h" #include "psdcalculator.h" +#include "kstvector.h" extern "C" void rdft(int n, int isgn, double *a); @@ -153,8 +154,6 @@ bool apodize, ApodizeFunction apodizeFxn, double gaussianSigma, PSDType outputType, double inputSamplingFreq) { - Q_UNUSED(interpolateHoles) // right now, interpolation for holes will be done to the input data before passing it in to calculatePowerSpectrum. we can get a speed increase if we change this in the future... - if (outputLen != calculateOutputVectorLength(inputLen, average, averageLen)) { KstDebug::self()->log(i18n("in PSDCalculator::calculatePowerSpectrum: received output array with wrong length."), KstDebug::Error); return -1; @@ -212,10 +211,23 @@ } // apply the PSD options (removeMean, apodize, etc.) - if (removeMean && apodize) { + // separate cases for speed- although this shouldn't really matter- the rdft should be the most time consuming step by far for any large data set. + if (removeMean && apodize && interpolateHoles) { for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { + _a[i_samp] = (kstInterpolateNoHoles(input, inputLen, i_samp + ioffset, inputLen) - mean)*_w[i_samp]; + } + } else if (removeMean && apodize) { + for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { _a[i_samp] = (input[i_samp + ioffset] - mean)*_w[i_samp]; } + } else if (removeMean && interpolateHoles) { + for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { + _a[i_samp] = kstInterpolateNoHoles(input, inputLen, i_samp + ioffset, inputLen) - mean; + } + } else if (apodize && interpolateHoles) { + for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { + _a[i_samp] = kstInterpolateNoHoles(input, inputLen, i_samp + ioffset, inputLen)*_w[i_samp]; + } } else if (removeMean) { for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { _a[i_samp] = input[i_samp + ioffset] - mean; @@ -224,6 +236,10 @@ for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { _a[i_samp] = input[i_samp + ioffset]*_w[i_samp]; } + } else if (interpolateHoles) { + for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { + _a[i_samp] = kstInterpolateNoHoles(input, inputLen, i_samp + ioffset, inputLen); + } } else { for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { _a[i_samp] = input[i_samp + ioffset]; @@ -239,7 +255,7 @@ } } - double frequencyStep = inputSamplingFreq/_awLen; + double frequencyStep = .5*(double)inputSamplingFreq/(double)(outputLen-1); //normalization factors which were left out earlier for speed. // - 2.0 for the negative frequencies which were neglected by the rdft //FIXME: double check.
SVN commit 554017 by dhanson: CCBUG:129168 backwards compatibility. not storing _averageLen is kind of strange though... M +3 -2 kstcsd.cpp --- trunk/extragear/graphics/kst/src/libkstmath/kstcsd.cpp #554016:554017 @@ -181,6 +181,7 @@ double *tempOutput, *input; int tempOutputLen = PSDCalculator::calculateOutputVectorLength(_windowSize, _average, _averageLength); + _PSDLen = tempOutputLen; tempOutput = new double[tempOutputLen]; input = inVector->value(); @@ -227,7 +228,7 @@ ts << l2 << "<vectag>" << QStyleSheet::escape(_inputVectors[INVECTOR]->tagName()) << "</vectag>" << endl; ts << l2 << "<sampRate>" << _frequency << "</sampRate>" << endl; ts << l2 << "<average>" << _average << "</average>" << endl; - ts << l2 << "<fftLen>" << _averageLength << "</fftLen>" << endl; + ts << l2 << "<fftLen>" << int(ceil(log(double(_PSDLen*2)) / log(2.0))) << "</fftLen>" << endl; ts << l2 << "<removeMean>" << _removeMean << "</removeMean>" << endl; ts << l2 << "<apodize>" << _apodize << "</apodize>" << endl; ts << l2 << "<apodizefxn>" << _apodizeFxn << "</apodizefxn>" << endl; @@ -345,7 +346,7 @@ } int KstCSD::length() const { - return _averageLength; + return _averageLength; } void KstCSD::setLength(int in_length) {
SVN commit 554018 by dhanson: CCBUG:129168 backwards compatibility. not storing _averageLen is kind of strange though... M +1 -0 kstcsd.h --- trunk/extragear/graphics/kst/src/libkstmath/kstcsd.h #554017:554018 @@ -103,6 +103,7 @@ double _gaussianSigma; int _windowSize; int _averageLength; + int _PSDLen; QString _vectorUnits; QString _rateUnits;
SVN commit 554020 by dhanson: CCBUG:129168 have KstPSD use PSDCalculator. use enumed apodization functions. M +1 -1 kst/main.cpp M +1 -1 libkstapp/datawizard.ui.h M +3 -1 libkstapp/kstiface_impl.cpp M +2 -2 libkstapp/kstpsddialog_i.cpp M +51 -216 libkstmath/kstpsd.cpp M +13 -15 libkstmath/kstpsd.h
hi george, re: my earlier email. i can now confirm the problem. the power spectrum produced by psdcalculator is a bit too high at the low frequency end. good eye. i've identified the issue- i divided by an incorrect variable in the 'new' psdcalculator. i'll commit a fix. duncan. (the blank plots were because gyrodata.dat wasn't being located- has the way in which kst attaches data files changed recently?) On Tue, 2006-07-11 at 19:02 -0400, George Staikos wrote: > Seems to me that PSDs are not working right anymore. demo.kst has quite an > odd look to it. Can anyone else confirm? >
SVN commit 561384 by dhanson: CCBUG:129168 fix incorrect division in psdcalculator M +1 -1 psdcalculator.cpp --- trunk/extragear/graphics/kst/src/libkstmath/psdcalculator.cpp #561383:561384 @@ -199,7 +199,7 @@ for (i_samp = 0; i_samp < currentCopyLen; i_samp++) { mean += input[i_samp + ioffset]; } - mean /= (double)_awLen; // _awLen != 0 b/c outputLen != 0. + mean /= (double)currentCopyLen; } // apply the PSD options (removeMean, apodize, etc.)
*** Bug has been marked as fixed ***.