Test fixes

[ellert]

Here are some fixes to the tests.

[ellert]

The 8f1e750 commit by @dpiparo caused a conflict with this pull request. I am not sure how to address the conflict, since as far as I can see the commit to the root master doesn't fix the issue. As far as I can see all it does is add some debug output, but still fail the tests. Or did I miss something?

[dpiparo]

Hi,

thanks for this huge PR: you really put forward quite some solid work about several tests - thanks. For what platform (os, compiler, architecture) were you exactly fixing them? We run them on the platforms listed here: http://cdash.cern.ch/index.php?project=ROOT

My comments, file by file, starting from the ones which might need some discussion before being incorporated in the master. The ones labelled with OK, all but 5, listed at at the end, are for me ready to go in.

• math/mathmore/test/testSpecFunc.cxx: we do not see this failure on 64 and 32 bits platforms. On what architecture/compiler did the test fail?
• math/mathmore/test/testSpecFunc.cxx: Nice update for coding conventions. My changes were aiming to fix 32 bits builds. Is this the case for you too?
• test/stressGraphics.ref: I think this is something to be commented by @couet
• test/stressMathCore.cxx: This was not failing for us. The question asked above about the failing platform still holds.
• test/stressTMVA.cxx: I am not sure I get the advantage of trying 2 paths and then falling back on the file on the web: was this a problem for you? Putting @lmoneta in the loop.

Files ready to go:

• THistPainter.cxx: OK
• CMakeLists: OK
• test/stressGraphics.cxx: OK
• test/stressHistogram.cxx: OK
• test/stressRooFit.cxx: OK
• tmva/pymva/test/Classification.C: OK
• tmva/rmva/test/Classification.C: OK
• tutorials/hist/th2polyUSA.C: OK
• tutorials/mlp/mlpHiggs.C: OK
• tutorials/quadp/portfolio.C: OK
• tutorials/tmva/TMVAClassification.C: OK
• tutorials/tmva/TMVAClassificationApplication.C: OK
• tutorials/tmva/TMVARegression.C: OK
• tutorials/tmva/TMVARegressionApplication.C: OK

[ellert]

Sorry for the delay. I should have made notes as to why exactly I added each of the changes, but I didn't - so i had to run some tests without the changes applied. Here we go:

For Fedora rawhide i686:

• gcc-c++-6.1.1-2.fc25.i686.rpm
• glibc-2.23.90-23.fc25.i686

The following tests FAILED:
45 - mathmore-testSpecFunc (Failed)
82 - test-stressgraphics (Failed)
83 - test-stressgraphics-interpreted (Failed)
115 - test-TFormulaTests (Failed)

Details:

• mathmore-testSpecFunc

laguerre(4, 1.)                                   :      FAILED
Discrepancy in laguerre(4, 1.) () :
  -0.625000000000000555 != -0.625 discr = 1   (Allowed discrepancy is 4.44089209850062616e-16)

• test-stressgraphics
  Failed subtests - 5 9 13 15 37 39 41
• test-stressgraphics-interpreted
  Failed subtests - 5 9 13 15 37 39 41
• test-TFormulaTests
  failed tests are : 18 19 23 28

For Fedora 23 i686

• gcc-c++-5.3.1-6.fc23.i686
• glibc-2.22-17.fc23.i686

The following tests FAILED:
45 - mathmore-testSpecFunc (Failed)
82 - test-stressgraphics (Failed)
83 - test-stressgraphics-interpreted (Failed)
94 - test-stressmathcore (Failed)
115 - test-TFormulaTests (Failed)

Details:

• mathmore-testSpecFunc

laguerre(4, 1.)                                   :  FAILED 
Discrepancy in laguerre(4, 1.) () :
  -0.625000000000000555 != -0.625 discr = 1   (Allowed discrepancy is 4.44089209850062616e-16)

• test-stressgraphics
  Failed subtests: 5 8 9 10 11 12 13 14 15 16 37 39 41
• test-stressgraphics-interpreted
  Failed subtests: 5 8 9 10 11 12 13 14 15 16 37 39 41
• test-stressmathcore

PtEtaPhiMVector operations              
Discrepancy in PtEtaPhiMVector operations() :
  62919.121533774116 != 62919.1608736201233 discr = 2   (Allowed discrepancy is 2.22044604925031308e-07)
        ............ FAILED 

PxPyPzMVector operations                
Discrepancy in PxPyPzMVector operations() :
  62919.1215337740286 != 62919.1608736201233 discr = 2   (Allowed discrepancy is 2.22044604925031308e-07)
        ............ FAILED 

• test-TFormulaTests
  failed tests are : 18 19 23 27 28

Regarding test/stressTMVA.cxx: there are several tests that use the tmva_class_example.root and tmva_reg_example.root files as input:

• test/stressTMVA.cxx
• tutorials/tmva/TMVAClassificationApplication.C
• tutorials/tmva/TMVAClassification.C

Before they all used slightly different logic for what locations to try and in what order, which didn't make sense to me. I therefore harmonized the usage so that they all do the same thing.

[couet]

I see you access http://root.cern.ch/files/usa.root using CACHEREAD in all places where it is used (tutorial, stressgraphics, THistpainter) ... fine with me ... but I do not understand why you changed stressgraphics.ref .... ?

[couet]

Can you send me the stressGraphics output you get (before the changes you did in stressgraphics.ref) ?

[ellert]

This is the output form Fedora 23 i686. The output differs between different releases and different architectures, but I think this is the one with the most failures.
stressgraphics.txt

[couet]

I have adjusted stress graphics.ref accordingly. Thanks.

[couet]

BTW: I have noticed that the stressGraphics output contains now the following line because of your changes:

Info in TFile::OpenFromCache: using local cache copy of http://root.cern.ch/files/usa.root [./files/usa.root]

This will make roottest fail....

[ellert]

I am not familiar with roottest. This output line does not make ctest fail. Why are they different?

[couet]

As far as I know roottest compares the output of of stressGraphics to a reference. Having this extra printout will make it different from the reference. My guess is that we will get a test failure..

[peremato]

test-stressgraphics is not comparing the output. It is looking for "FAILED|Error in" in the output.

[couet]

Ah ok .. so in that case this extra printout should not arm....
Sorry for the noise.

[ellert]

As I wrote before the Fedora 23 i686 was not the only failures in stress graphics, only the one with the most of them. I tried rerunning usung the new reference from master and had 4 failures in epel 7:
stressgraphics-epel7.txt

There was also one failure in Fedora 24, but it was the same as one of the 4 failures above.

[couet]

I did the needed changes in stressGraphics.ref. Thanks.

[pcanal]

@ellert I am working on the merge. Several things have already been done as part of the master so there was (as expected) several merge conflict. I pick resolutions that can be seen at: https://github.com/pcanal/root/tree/ellert-test-fixes

Can you please verify that they are complete/correct for you?

Thanks.

[ellert]

If I merge the master (without the last two commits) into this test-fixes branch and then check the differences with your branch, only the test/TFormulaParsingTests.h file differs.

I think the change made in master to that file attempted to fix some of the same test failures I attempted to fix, but not all of them. An fpEqual function was introduced to do comparisons between numbers, but unless the 3rd optional argument is given, that function only does normal operator== between the numbers. And only in one of the calls does it actually set this option.

In my version I used a similar approach, but I used TMath::AreEqualAbs instead of writing a new function. I chose this function for consistency since it was already used by a few of the tests in the file. I added calls to TMath::AreEqualAbs to 6 places in the file, not only 1 - though one of the six is the same as the one place where the third option to fpEqual is used.

Whether to use the existing TMath::AreEqualAbs or write a new similar function is of course a matter of taste, but since at present in master the third argument to fpEqual is only used at one single place the implementation in master is incomplete and some of the tests will fail in some architectures.

[ellert]

The log of the failing stressMathCore test on Fedora 26 i686 says:

XYZVector operations                    		..............
RhoEtaPhiVector operations              
Discrepancy in RhoEtaPhiVector operations() :
  122283.610163202422 != 122283.610163183446 discr = 1   (Allowed discrepancy is 1.33226762955018785e-13)
		............ FAILED 

[ellert]

This pull request is now quite old. It contains various fixes all related to the runnng to the test suite. Some of the changes originally proposed here were cherry picked and merged, while others remain unresolved. New test related problems were added over time, and since the pull request is so old it has been necessary to merge the master branch in order to resolve merge conflicts several times. All this makes it hard to review the proposed changes.

For this reason I have split the proposed changes in this pull request into several new ones, each of which addresses one singe issue. This will hopefully make it easier to review.

The new pull requests replacing this one are: #400, #401, #402, #403, #404 and #405.

I now close this one.