Skip to content
Snippets Groups Projects

Execution of reconstruction from timeslice with steering class and minimal run macro. Refs #2275.

Merged Volker Friese requested to merge v.friese/cbmroot:reco_steer into master

This MR continues on the integration of the timeslice processing in !766 (merged) by transferring macro code into compiled code. It introduces a steering class CbmReco wrapping FairRunOnline, in the spirit of CbmSimulation and CbmDigitization. So, the coupling to FairRun stays, but is hidden from the user. Internally, also FairTasks are continued to be used and exchange data through the ROOT tree.

Despite the internals not being changed a lot, this goes a step further towards a single executable. The execution macro (macro/reco/reco_steer.C) how merely serves for the configuration. A configuration struct CbmRecoConfig was introduced along CbmReco.

If we want to go for an executable, there will be missing but two steps:

  • Make CbmReco executable (build system)
  • Read the configuration from a text file (YAML or the like).

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Volker Friese requested review from @p.-a.loizeau

    requested review from @p.-a.loizeau

  • Volker Friese added 1 commit

    added 1 commit

    • cef34342 - Add missing include directory to CMakeLists.txt

    Compare with previous version

  • Volker Friese added 1 commit

    added 1 commit

    Compare with previous version

  • I fear the builds for jun19 will still fail due to the use of make_unique which is available only from C++14 onward while jun19 is sticking to c++11 (at least from what the Ubuntu 20.04 compiler told me when I tried it).


    At this point, I would actually use an std::vector<std::string> as member for the source and then

    • a call to CbmSourceTs(std::vector<std::string> fileNames); in CbmReco.cxx
    • two constructors for CbmReco, one straight with std::vector<std::string> fileNames and another with an std::string source to keep the single string option, this way the macro should not need any modification

    The following diff would to these changes and compiled for me under Ubuntu 20.04 with the apr20p2 and v18.6.7:

    diff --git a/reco/steer/CbmReco.cxx b/reco/steer/CbmReco.cxx
    index 64770bf9..d1ac96cd 100644
    --- a/reco/steer/CbmReco.cxx
    +++ b/reco/steer/CbmReco.cxx
    @@ -23,8 +23,15 @@ using std::make_unique;
     
     
     // -----   Constructor   ------------------------------------------------------
    -CbmReco::CbmReco(TString source, TString outFile, int32_t numTs, const CbmRecoConfig& config)
    -  : fInputFileName(source)
    +CbmReco::CbmReco(std::vector< std::string > sources, TString outFile, int32_t numTs, const CbmRecoConfig& config)
    +  : fSourceNames(sources)
    +  , fOutputFileName(outFile)
    +  , fNumTs(numTs)
    +  , fConfig(config)
    +{
    +}
    +CbmReco::CbmReco(std::string source, TString outFile, int32_t numTs, const CbmRecoConfig& config)
    +  : fSourceNames{source}
       , fOutputFileName(outFile)
       , fNumTs(numTs)
       , fConfig(config)
    @@ -42,10 +49,10 @@ int32_t CbmReco::Run()
       timer.Start();
     
       // --- Input source
    -  auto source = make_unique<CbmSourceTs>(fInputFileName.Data());
    -  if (source) LOG(info) << "Reco: Using source " << fInputFileName.Data();
    +  auto source = make_unique<CbmSourceTs>(fSourceNames);
    +  if (source) LOG(info) << "Reco: Using sources " << ListInputs();
       else {
    -    LOG(error) << "Reco: Could not open source " << fInputFileName.Data() << "; aborting.";
    +    LOG(error) << "Reco: Could not open sources " << ListInputs() << "; aborting.";
         return -1;
       }
     
    @@ -118,5 +125,15 @@ int32_t CbmReco::Run()
       return fNumTs;
     }
     // ----------------------------------------------------------------------------
    +std::string CbmReco::ListInputs() const
    +{
    +  std::string concstr = "{";
    +  for (auto & source : fSourceNames) {
    +    concstr += source + ", ";
    +  }
    +  concstr += "}";
    +  return concstr;
    +}
    +// ----------------------------------------------------------------------------
     
     ClassImp(CbmReco)
    diff --git a/reco/steer/CbmReco.h b/reco/steer/CbmReco.h
    index 7a4aace9..9106f826 100644
    --- a/reco/steer/CbmReco.h
    +++ b/reco/steer/CbmReco.h
    @@ -57,14 +57,21 @@ public:
       /** @brief Default constructor **/
       CbmReco() {};
     
    -
       /** @brief Standard constructor
    +   ** @param source  Vector of names of input files or input sources
    +   ** @param outFile Name of output file
    +   ** @param numTs   Number of timeslices to process. If negative, all available will be used.
    +   ** @param config  Configuration
    +   **/
    +  CbmReco(std::vector< std::string > sources, TString outFile, int32_t numTs, const CbmRecoConfig& config);
    +
    +  /** @brief Standard constructor for single source
        ** @param source  Name of input file or input source
        ** @param outFile Name of output file
        ** @param numTs   Number of timeslices to process. If negative, all available will be used.
        ** @param config  Configuration
        **/
    -  CbmReco(TString source, TString outFile, int32_t numTs, const CbmRecoConfig& config);
    +  CbmReco(std::string source, TString outFile, int32_t numTs, const CbmRecoConfig& config);
     
     
       /** @brief Destructor **/
    @@ -76,9 +83,13 @@ public:
        **/
       int32_t Run();
     
    +  /** @brief List all entries in the input vector
    +   ** @return String concatenating the sources names
    +   **/
    +  std::string ListInputs() const;
     
     private:
    -  TString fInputFileName  = "";
    +  std::vector< std::string > fSourceNames  = {};
       TString fOutputFileName = "";
       int32_t fNumTs          = -1;
       CbmRecoConfig fConfig   = {};
  • Volker Friese added 1 commit

    added 1 commit

    • 7ba20aa8 - Removed C++17 features; added handling of a list of sources, following PAL's suggestions.

    Compare with previous version

  • I fear the builds for jun19 will still fail due to the use of make_unique which is available only from C++14 onward while jun19 is sticking to c++11 (at least from what the Ubuntu 20.04 compiler told me when I tried it).

    Yes, I realised that. Gets more annoying the longer it takes...

    Thanks for the patch, is included in the latest commit.

  • Volker Friese added 1 commit

    added 1 commit

    • 78cbf870 - Moved new class CbmReco to reco/tasks to prevent building on systems without C++17.

    Compare with previous version

  • Volker Friese added 1 commit

    added 1 commit

    • f8887c5e - Remove dependence of CbmRecoSteer from CbmRecoTasks.

    Compare with previous version

  • Volker Friese added 7 commits

    added 7 commits

    • e335e610 - 1 commit from branch computing:master
    • d307f3ca - Execution of reconstruction from timeslice with steering class and minimal run macro. Refs #2275.
    • 1766464e - Add missing include directory to CMakeLists.txt
    • e2481e91 - Grmpfff...
    • 94b474c7 - Removed C++17 features; added handling of a list of sources, following PAL's suggestions.
    • 24708598 - Moved new class CbmReco to reco/tasks to prevent building on systems without C++17.
    • 27229597 - Remove dependence of CbmRecoSteer from CbmRecoTasks.

    Compare with previous version

  • Dear @f.uhlig, @p.-a.loizeau, @v.friese,

    you have been identified as code owner of at least one file which was changed with this merge request.

    Please check the changes and approve them or request changes.

  • Volker Friese resolved all threads

    resolved all threads

  • Florian Uhlig
  • Florian Uhlig resolved all threads

    resolved all threads

  • Jan de Cuveland
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading