MuchQA

added 3 commits

00424d65 - 1 commit from branch computing:master
648b9cbe - Cleaned up header files of MuchQA.
27a88a6d - Moved CbmMuchPointInfo to core/detectors/much/. Adjusted CMakeLists and...

added 4 commits

27a88a6d...4f3cbefd - 2 commits from branch computing:master
5a2d55b4 - Cleaned up header files of MuchQA.
c3d4712b - Moved CbmMuchPointInfo to core/detectors/much/. Adjusted CMakeLists and...

could you check the files using the tool include-what-you-use which you can install with brew. This tool will show you which includes/forward declarations are really needed.

You can find the usage instructions at https://github.com/include-what-you-use/include-what-you-use/blob/master/README.md#using-with-a-compilation-database.

After creating the compilation database by running cmake with the extra option -DCMAKE_EXPORT_COMPILE_COMMANDS=ON I use a command like the following from the CbmRoot command line

iwyu_tool.py -p <build_directory> sim/detectors/much/qa/CbmMuchTransportQa.cxx

include-what-you-use creates a detailed output with needed changes for the header and source file of the respective class.

Ok, I just did this. The include lists that this tool produces are much longer than mine. It suggests all of the includes that I have, but also a bunch of others that don't seem to be needed. I went through them line by line on Friday.

Most likely there are some nested includes in the header files which I include, which include-what-you-use does not detect, hence my list being shorter. Perhaps I am using the tool wrong? Should these extra includes be listed explicitly or should the list be kept short?

In my opinion the list of header files and forward declarations should be complete such that all really needed header files are included. This is what include-what-you-use (IWYU) does for you.

According to my version of include-what-you-use I am instructed to add the following lines to the header file of CbmMuchTransportQa

#include <Rtypes.h>      // for THashConsistencyHolder, ClassDef
#include <RtypesCore.h>  // for Int_t, Option_t
#include <TFolder.h>     // for TFolder
#include <vector>        // for vector
class TBuffer;
class TClass;
class TClonesArray;
class TMemberInspector;

Rtypes.h and RtypesCore.h are missing since in this headers the ClassDef macro and the ROOT data types are defined.
TFolder.h and vector are missing since in the header file there are objects of this type defined and used. The forward declaration of TClonesArray since a pointer of this type is defined. The forward declarations of TBuffer, TClass and TMemberInspector come in due to the code form the ClassDef macro. If you comment the line with the ClassDef the list of includes is reduced to

#include <RtypesCore.h>  // for Int_t, Option_t
#include <TFolder.h>     // for TFolder
#include <vector>        // for vector
class TClonesArray;

In my opinion the information you get from IWYU is correct. That compilation also works without the correct list of includes/forward declarations is in my opinion only pure luck since TFolder.h and vector are simply included one of the other included header files. For example vector is included by the chain CbmMuchTransportQa.h->FairTask.h->FairRootManager.h If this hidden and implicit include is removed in future from FairRootManager.h our code will not compile any longer even if we did no changes at all. This problem we had already several times when upgrading to new versions of FairRoot and/or ROOT.

Looking at the list of includes for the source file most of the newly needed includes are obvious since they are used directly in the code but some are not so straight forward to find. For example TAxis.h is include even if this you don't find TAxis it in the code. But it is used indirectly in the following code

    TH1F* h = fvFraction[i];
    h->SetStats(0);
    h->GetXaxis()->SetTitle("Station");

The same is true for TString.h which is not used directly but indirect in Form(...) which is defined in the TString header file.

The only includes I would be careful are

#include <stdlib.h> // for abs #include <sys/types.h> // for uint

since this could be OS dependent, so one would have to test this also on Linux.

To summarize, in my opinion the include statements should be minimal but complete without any indirect and hidden dependencies. The suggestions of IWYU are normally correct and could be added.

added 1 commit

94431e2e - CbmMuchQA: Updated headers according to iwyu.

Compare with previous version

Done. There were no problems with stdlib.h and types.h on my Ubuntu 20.04 installation.

Ok, now I have no idea what is going on. Some of the macos tests apparently failed. This is puzzling, as the code compiles and runs on my mac and "make test" also completes without errors. Also, I did not even change any of the offending files which the error message lists and the regular (non mac?) CbmRoot_Merge completes just fine.

This did not happen before my last changes to the header files. I don't see how changing the headers of QA tasks could have done this:

The following tests FAILED:

 81 - mcbm_reco_event_mcbm_beam_2020_03 (Timeout)
 82 - mcbm_hadron_analysis_mcbm_beam_2020_03 (Not Run)
 83 - mcbm_digi_mcbm_beam_2020_03 (Not Run)

I restarted the test already. Let see if it finishes now. I think it is because one of the tests run in a timeout.

Probably my system runs out of memory.

approved this merge request

assigned to @p.-a.loizeau

@d.smith,

Please rebase

added 20 commits

94431e2e...7b408972 - 17 commits from branch computing:master
7f001f48 - Cleaned up header files of MuchQA.
fb651a02 - Moved CbmMuchPointInfo to core/detectors/much/. Adjusted CMakeLists and...
82fb2e94 - CbmMuchQA: Updated headers according to iwyu.

Compare with previous version

Ok, did. Pipeline is running again. I guess this is normal.

Yes, this is normal. Whenever there is an update of the MR the pipeline has to rerun.

Passed

merged

MuchQA

Merge request reports

Activity