[Clam-devel] test changes after refactoring of InstantTunningEstimator

[Roman Goj]

On 7/10/07, David García Garzón <dgarcia at iua.upf.edu> wrote:
> On Tuesday 10 July 2007 11:39:39 Roman Goj wrote:
> > Hi!
> >
> > After yesterday's chat learning session with David, I'm sending very
> > small patches now :)
> >
> > The next few patches should make the tests in
> > InstantTunningEstimatorTest use the std::vector version of doIt in
> > InstantTunningEstimator and change the positions in all the test to
> > use the new conventions of numbering the bins in a semitone (was: each
> > semitone has bins 0,1,2 and is now: 0, 1/3, 2/3 or similar...). The
> > TODO's are in my GSoC space on the Devel/Chord Extraction TODOs page
> > if anyone's interested :)
>
> Nice. Tests in green? No changes needed?

Yep, here everything works smoothly. Of course I also tested if
everything is as expected if I change some values, so that an error is
printed and it also works, so green here :)

> > This patch is:
> > Change assertFoundCenterIs to use the vector<pair> interface
> >
> > Btw. there's a std::vector< std::pair<double,double> > and in this
> > patch I had to initialize it from two arrays of doubles and did this
> > in a loop:
> >
> > unsigned nPeaks;
> > double * peakPositions;
> > double * peakValues;
> > (...)
> > std::vector< std::pair<double,double> > peaks;
> > for(int i=0; i<nPeaks; ++i)
> > {
> >      peaks.push_back(
> > std::pair<double,double>(peakPositions[i],peakValues[i])); }
> >
> > Am I missing some other obvious way of making a vector of pairs out of
> > two arrays of doubles? Guess not, or...?
>
> std::make_pair(peakPositions[i],peakValues[i])

oh, right :) should I change? I guess the constructor isn't that bad either?

> > Also - David - I didn't ask about this yesterday, but:
> >
> > Should I also modify the code in the tests themselves, so that the
> > assertFoundCenterIs changes from:
> > assertFoundCenterIs(double,double,unsigned,double *, double *)
> > to
> > assertFoundCenterIs(double,double,unsigned,std::vector<
> > std::pair<double,double> >
> > ?
>
> No, for testing is great to have plain C arrays. We can provide literals.
>
> > Then the code I asked about above could disappear from assertFoundCenterIs.
>
> But think, then you should fill the vector in every test.

well... we have to fill the C arrays anyway... and then we fill the
vector with the values from the arrays...
I agree though that the code looks a bit worse (I did this already in
the last two tests to use doIt( vector) ) :


-               double positions1[]={0.0, 3.0};
-               double values1   []={8.0, 8.0};

+               peaks1.push_back( std::pair<double,double>(0.0, 8.0) );
+               peaks1.push_back( std::pair<double,double>(1.0, 8.0) );

much less readable...

so - ok, skip this idea

shall I tick the first three TODO's though and do the rest, everything ok? :)