Page MenuHomeHEPForge

Modernize EvtGen
Open, NormalPublic

Description

Incorporate C++ modernization changes from Gerhard Raven (LHCb):

https://gitlab.cern.ch/lhcb/Gauss/merge_requests/382

Initial import of all changes made to the modernize branch:

https://phab.hepforge.org/rEVTGENb240d9114f86

Related Objects

Event Timeline

jback created this task.Dec 21 2018, 6:49 PM
jback triaged this task as Normal priority.
jback added a comment.Dec 21 2018, 6:51 PM

There are compilation problems, specifically private copy constructors in EvtItgPtrFunction and related classes, giving errors when auto and make_unique are used in EvtBtoXsgammaKagan, for example. Similar issues occur for
EvtRareLbToLll, EvtVubBLNP and EvtVubNLO. Could be compiler-version dependent?

Not added to this repository yet is the LHCb-only EvtBsMuMuKK model.

We may also not need EvtNoRadCorr.hh.

Added missing EvtHQET2FF.cpp bug fix for a normalisation term (f0m) that was in the LHCb version but not copied over.

Could also think about importing cmake changes from Tom:

https://phab.hepforge.org/T12

New models need modernization: BLLNuL, BLLNuLAmp and BToDiBaryonlnupQCD, BToDiBaryonlnupQCDFF, EvtSLDiBaryonAmp.

jback added a comment.Dec 21 2018, 7:22 PM

LHCb repository uses report() for print statements while our version uses EvtGenReport(). Used the following sed-like commands to change these lines:

sed -i 's/report(ERROR/EvtGenReport(EVTGEN_ERROR/g' fileName

jback claimed this task.Dec 21 2018, 7:34 PM
kreps added a comment.Dec 21 2018, 7:37 PM

@jback, which version of g++ you use? I think we might need at least C++14 for code changes Gerhard did. Using g++ 6.2 should work, g++ 4 will definitely not.

kreps added a comment.Dec 22 2018, 7:39 AM

@jback, I think you are still compiling with C++11 using old build system. We will need CMake as soon as we are removing EvtGen from LHCb repository, so lets review Tom's changes for that and merge that into master. Then we can rebase modernize branch from master to get CMake build system into it and by that it should be basically automatic as Tom put at least C++14 into that.

jback added a comment.Dec 24 2018, 3:52 PM

I tried gcc 6.3.0 and the configure script has been modified to enable C++14, although the corresponding environment variable is still called CPP11.

The script also continues to pass the "-D EVTGEN_CPP11" boolean flag, but this is not in fact needed anymore; it was only used in EvtMTRandomEngine and this class has been modified to not require the flag.

I agree that we should move to the cmake system as soon as we can.

FYI, quite a nice way of getting a consistent environment for testing these things is to do something like the following (this should work on any system with cvmfs installed):
source /cvmfs/sft.cern.ch/lcg/views/setupViews.sh LCG_94 x86_64-centos7-gcc8-opt
(I'm assuming you're in bash here)
You can obviously modify the LCG version and the platform etc. as you wish.
NB that for the above one you'll need to use C++17
One other nice feature of using LCG views to set the environment is that these LCG releases contain some of our dependencies, so we don't have to mess around building them ourselves necessarily. They seem to get automatically picked up by cmake - as least this worked for HepMC2 when I tried it just now.

jback added a comment.Jan 30 2019, 2:45 PM

I think I've fixed all of the compilation errors and warnings, and have also added the cmake branch changes as well. To build this modernize branch using cmake, do:

git clone ssh://vcs@phab.hepforge.org/source/evtgen.git evtgen.git
cd evtgen.git
git checkout modernize
cd ..
mkdir evtgen.build
cd evtgen.build

cmake -DCMAKE_INSTALL_PREFIX=../evtgen ../evtgen.git -DHEPMC2_ROOT_DIR=HepMCDir -DPYTHIA8_ROOT_DIR=pythiaDir -DPHOTOSPP_ROOT_DIR=photosDir -DTAUOLAPP_ROOT_DIR=tauolaDir -DEVTGEN_PYTHIA=on -DEVTGEN_PHOTOS=on -DEVTGEN_TAUOLA=on

make
make install

A few thoughts on the modernisation and general tidy-ups:

  • We should try to follow the "rule of all or nothing" with regard to the special member functions (i.e. destructor, copy constructor, copy assignment operator, move constructor, move assignment operator), and make good use of providing default values for data members directly in the header files, constructor delegation, etc.
  • It would be good to review the use of pointers/references/smart-pointers, in particular in the interface, i.e. public member function arguments, returns etc.
  • We should probably decide on a code format convention, construct a clang-format config file to reflects that convention, and apply it to all files (it might even be possible to construct a hook on the repository that checks for conformance on push)
  • It would be good to test everything on both gcc and clang so that we catch as many problems as we can
WARNING: about the readiness of the CMake stuff - see my comment T12#228 on the CMake task
tlatham moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 5 2019, 9:11 PM
tlatham changed the edit policy from "All Users" to "Restricted Project (Project)".Feb 25 2020, 10:47 AM
tlatham edited subscribers, added: Restricted Project; removed: kreps, tlatham, jback.
tlatham changed the visibility from "All Users" to "Public (No Login Required)".Jul 27 2020, 10:08 AM
tlatham changed the edit policy from "Restricted Project (Project)" to "Restricted Project (Project)".Tue, Oct 6, 10:18 AM