Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output object registered from generator class is not saved to ntuple. #1183

Closed
mgoncerz opened this issue May 17, 2022 · 22 comments
Closed

Output object registered from generator class is not saved to ntuple. #1183

mgoncerz opened this issue May 17, 2022 · 22 comments
Assignees
Labels
need-triage Missing milestone assignment

Comments

@mgoncerz
Copy link

I'm trying to register, using FairRootManager::Instance()->RegisterAny("event", m_event, kTRUE), a simple object containing event weights from a Monte Carlo generator, but the output is not saved to the final ntuple.

I'm doing it inside an Init method of a custom generator class, deriving from FairGenerator (which I'm sure is actually ran, because it sets other things as well). The generator is added in a production macro like this:

FairPrimaryGenerator* primGen = new FairPrimaryGenerator();
primGen->AddGenerator(generator);
run->SetGenerator(primGen);

Where run is FairRunSim. The same method works fine for me within FairTasks, running both under FairRunSim and FairRunAna.

@karabowi
Copy link
Collaborator

I have just added a dummy struct to the FairRoot's FairBoxGenerator, and the corresponding branch is saved in the output tree no problem. Can you share the code?

@mgoncerz
Copy link
Author

mgoncerz commented May 18, 2022

Sure. I understand it's a bit convoluted, so please let me know if you want me to and I can try to simplify the problem a bit, if the problem is not immediately obvious.

We have a main generator class that combines different generators for the beam track and interaction vertices, that's the one inheriting from FairGenerator. It's initialization method is defined here: https://gitlab.cern.ch/mgoncerz/fairmuone/-/blob/nlo_implementation_test/MUonE/MUonEGenerators/MUonEGenerator.cxx#L195, where by the end m_vertexGenerator->Init() is called and is a method defined by me in a base class for interaction vertex generators.

This method is defined for the interface of a Mesmer generator here: https://gitlab.cern.ch/mgoncerz/fairmuone/-/blob/nlo_implementation_test/MUonE/MUonEGenerators/VertexGenerators/Mesmer/MUonEVertexGeneratorMesmer.cxx#L37, I'm sure this is run, because the steering card file defined there is being created.

The object I'm trying to save (https://gitlab.cern.ch/mgoncerz/fairmuone/-/blob/nlo_implementation_test/MUonE/MUonEGenerators/VertexGenerators/Mesmer/MUonEVertexGeneratorMesmer.h#L43) is of type defined by external Fortran/c++ interface here https://gitlab.cern.ch/muesli/nlo-mc/mue/-/blob/master/code/MuEtree.h#L156
As the repository requires cern account, here's a copy:

class Event {

  public:
    Int_t RunNr;
    Long_t EventNr;
    Double_t wgt_full, wgt_norun, wgt_lep, wgt_LO, wgt_NLO;
    static const uint NC = 11;
    std::array<Double_t, NC> coef;
    Particle muin;
    std::vector<Particle> fspart;

    Event():
    RunNr(0),EventNr(0),wgt_full(0),wgt_norun(0),wgt_lep(0),wgt_LO(0),wgt_NLO(0)
    {};

    bool Read(std::ifstream &, bool read_coef=true, bool debug=false);
    void Print() const;

    int GenerateEvent_mesmer(double* pmu);
    
    virtual ~Event(){};
    
    ClassDef(Event,7)
  };

The linking and everything works fine, as other classes from the same interface work correctly, but just in case - the package is set up to be cloned and compiled during the compilation of our software in this cmake file -- https://gitlab.cern.ch/mgoncerz/fairmuone/-/blob/nlo_implementation_test/MUonE/MUonEGenerators/VertexGenerators/Mesmer/CMakeLists.txt

The macro we're using for production is defined here -- https://gitlab.cern.ch/mgoncerz/fairmuone/-/blob/nlo_implementation_test/MUonE/macros/runProductionJob.C. Most of the code is just configuration of different steps and checks, the important part is from line 26 to 32, 65-75 and 86.

I'm doing something exactly the same for our digitization task here - https://gitlab.cern.ch/mgoncerz/fairmuone/-/blob/nlo_implementation_test/MUonE/MUonEDigitization/Calorimeter/MUonECalorimeterDigitization.cxx#L128 and it works flawlessly.

The content of our output file looks like this (as you can see, no MesmerEvent is not saved (unless it's hidden somewhere, but I can't find it):
image

@mgoncerz
Copy link
Author

This is probably a better thing to show, as I think it contains all registered branches (also those that are only stored in memory). The one from generator is still not there.
image

@mgoncerz
Copy link
Author

mgoncerz commented May 18, 2022

I've tried to narrow the problem down and added a simple struct in our main generator class (MUonEGenerator). Just a:

class test {

	public:
		test(){};

		Int_t testint = 5;
		ClassDef(test,2)	
};

It should be equivalent to your test.

I've included it in the generator class as a pointer:
test* t;
created it in generator class constructor with:
t = new test();

registered in Init() with
FairRootManager::Instance()->RegisterAny("test", t, kTRUE);
outside of any ifs etc. Just in the beginning of the Init() method.

and also added line
#pragma link C++ class test+;
to the link def file.

It still doesn't show in the ntuple, which makes me think the problem may lie in how we use the generator class in the macro maybe?

@karabowi
Copy link
Collaborator

Hi Mateusz, can you try to declare the object t as the member of the generator class?
and then define it in the generator constructor?

@karabowi
Copy link
Collaborator

karabowi commented May 19, 2022

I had a closer look at your classes, and it looks that it should be working. Are you sure, that the MUonEVertexGeneratorMesmer::Init() is really called?

The git diff to my dummy example shows

diff --git a/generators/FairBoxGenerator.cxx b/generators/FairBoxGenerator.cxx
index 25d4f6b45..b3a8a11ae 100644
--- a/generators/FairBoxGenerator.cxx
+++ b/generators/FairBoxGenerator.cxx
@@ -30,6 +30,7 @@
 
 #include "FairLogger.h"
 #include "FairPrimaryGenerator.h"
+#include "FairRootManager.h"
 
 #include <TDatabasePDG.h>
 #include <TMath.h>
@@ -62,6 +63,7 @@ FairBoxGenerator::FairBoxGenerator()
     , fPtRangeIsSet(0)
     , fPRangeIsSet(0)
     , fEkinRangeIsSet(0)
+    , fEle(new ele{0,1})
 {
     // Default constructor
 }
@@ -89,6 +91,7 @@ FairBoxGenerator::FairBoxGenerator(Int_t pdgid, Int_t mult)
     , fPtRangeIsSet(0)
     , fPRangeIsSet(0)
     , fEkinRangeIsSet(0)
+    , fEle(new ele{0,1})
 {
     // Constructor. Set default kinematics limits
     SetPDGType(pdgid);
@@ -124,6 +127,8 @@ Bool_t FairBoxGenerator::Init()
         LOG(fatal) << "FairBoxGenerator: PDG " << GetPDGType() << " not defined";
     }
 
+    FairRootManager::Instance()->RegisterAny("elee", fEle, kTRUE);
+
     if (fPhiMax - fPhiMin > 360) {
         LOG(fatal) << "FairBoxGenerator:Init(): phi range is too wide: " << fPhiMin << "<phi<" << fPhiMax;
     }
@@ -161,6 +166,8 @@ Bool_t FairBoxGenerator::ReadEvent(FairPrimaryGenerator* primGen)
 
     Double32_t pabs = 0, phi, pt = 0, theta = 0, eta, y, mt, px, py, pz = 0;
     GenerateEventParameters();
+    fEle->a = gRandom->Integer(100);
+    fEle->b = GetMultiplicity();
     // Generate particles
     for (Int_t k = 0; k < GetMultiplicity(); k++) {
         phi = gRandom->Uniform(fPhiMin, fPhiMax) * TMath::DegToRad();
diff --git a/generators/FairBoxGenerator.h b/generators/FairBoxGenerator.h
index 90b97c591..a3e733f27 100644
--- a/generators/FairBoxGenerator.h
+++ b/generators/FairBoxGenerator.h
@@ -41,6 +41,11 @@
 
 class FairPrimaryGenerator;
 
+struct ele{
+    int a;
+    int b;
+};
+
 class FairBoxGenerator : public FairBaseMCGenerator
 {
   public:
@@ -134,6 +139,7 @@ class FairBoxGenerator : public FairBaseMCGenerator
     FairBoxGenerator& operator=(const FairBoxGenerator&) = default;
  
   private:
+    ele* fEle;
     Double32_t fPtMin, fPtMax;         // Transverse momentum range [GeV]
     Double32_t fPhiMin, fPhiMax;       // Azimuth angle range [degree]
     Double32_t fEtaMin, fEtaMax;       // Pseudorapidity range in lab system
diff --git a/generators/GenLinkDef.h b/generators/GenLinkDef.h
index 5fa5f680f..fb9d6bdbd 100644
--- a/generators/GenLinkDef.h
+++ b/generators/GenLinkDef.h
@@ -23,4 +23,6 @@
 #pragma link C++ class  FairYPtGenerator+;
 #pragma link C++ class  FairBaseMCGenerator+;
 
+#pragma link C++ class  ele;
+
 #endif

And the output file has the correct branch

*............................................................................*
*Branch  :elee                                                               *
*Entries :       10 : BranchElement (see below)                              *
*............................................................................*
*Br   45 :a         : Int_t                                                  *
*Entries :       10 : Total  Size=        669 bytes  One basket in memory    *
*Baskets :        0 : Basket Size=      32000 bytes  Compression=   1.00     *
*............................................................................*
*Br   46 :b         : Int_t                                                  *
*Entries :       10 : Total  Size=        669 bytes  One basket in memory    *
*Baskets :        0 : Basket Size=      32000 bytes  Compression=   1.00     *
*............................................................................*

@ChristianTackeGSI
Copy link
Member

Just wondering (I am just learning FairRoot):

I think

LOG(info) << "CREATING BRANCH " << iter.first;
…
LOG(info) << "Creating branch for " << iter.first.c_str() << " with address " << obj;

( https://github.com/FairRootGroup/FairRoot/blob/master/base/sink/FairRootFileSink.cxx#L283-L303 )
should generate some log messages with your branch name? Are there other log messages about your branch name maybe?

@karabowi
Copy link
Collaborator

karabowi commented May 20, 2022

You are totally right, Christian. Here's the message from my trivia example:

[INFO] FairMCApplication::InitGeometry: 0
[INFO] Simulation RunID: 1652951575
[INFO] CREATING BRANCH loce
[INFO] Creating branch for loce with address 0x7ffeebf6d150

 Calculating cross section tables, see gphysi.dat for more information

@mgoncerz
Copy link
Author

Thank you both for suggestions!

Hi Mateusz, can you try to declare the object t as the member of the generator class?
and then define it in the generator constructor?

That's what I did. Sorry for not stating it more clearly.

Just to make sure, I did


LOG(info) << "REGISTERING MESMER OUTPUT"; 
FairRootManager::Instance()->RegisterAny("MesmerEvent", m_event, kTRUE); 
LOG(info) << "REGISTERING MESMER OUTPUT";

in the beginning of the Init method and both lines appear in the log file.

I couldn't find any mention of neither Event or the test branch being created though, that is:

cat log.txt | grep -i 'branch'
[INFO] CREATING BRANCH CalorimeterDigiDeposit
[INFO] Creating branch for CalorimeterDigiDeposit with address 0x564c65d53368
[INFO] CREATING BRANCH ReconstructionOutput
[INFO] Creating branch for ReconstructionOutput with address 0x564c625f3c78

Full log here:
log.txt

@mgoncerz
Copy link
Author

It kind of seems like the generator is initialized too late for the I/O class to catch the registered branch. Could it be somehow related to the ordering in the production macro? As you've seen I've made it significantly more complex to account for all possible job configurations. Maybe the problem lies there?

@karabowi
Copy link
Collaborator

It might be a problem. Your generator initializes after the initialization of the sink. Since the branches are created during the Sink::Init() stage (CREATING BRANCH), the generators inform the FairRootManager about new branches too late (REGISTERING MESMER OUTPUT):

[INFO] Initializing event filter.
[INFO] Simulation RunID: 1653045065
[INFO] CREATING BRANCH CalorimeterDigiDeposit
[INFO] Creating branch for CalorimeterDigiDeposit with address 0x564c65d53368
[INFO] CREATING BRANCH ReconstructionOutput
[INFO] Creating branch for ReconstructionOutput with address 0x564c625f3c78
### INFO: TG4RootDetectorConstruction::ConstructSDandField finished

 hInelastic FTFP_BERT : threshold between BERT and FTFP is over the interval 
 for pions :   3 to 6 GeV
 for kaons :   3 to 6 GeV
 for proton :  3 to 6 GeV
 for neutron : 3 to 6 GeV

### Adding tracking cuts for neutron  TimeCut(ns)= 10000  KinEnergyCut(MeV)= 0
### Hadron physics constructed. 
### Step limiter physics constructed.
### Special Cuts constructed. 
### User particles physics constructed. 
### Processes mapped to VMC codes ok
TG4ComposedPhysicsList::ConstructProcess done
Available UI session types: [ tcsh, csh ]
[INFO] Initializing generator.
[INFO] REGISTERING MESMER OUTPUT
[INFO] REGISTERING MESMER OUTPUT
  *************************************************************

@karabowi
Copy link
Collaborator

I found out the problem. There is difference in behavior between Geant3 and Geant4.
I was running with Geant3 - the branch is created.
When I run with Geant4 (like you) - the branch is NOT created.
Will investigate it further.

@mgoncerz
Copy link
Author

Thanks! Just to confirm though, I set the sink after the generator is added.

@ChristianTackeGSI
Copy link
Member

@karabowi If this is a real regression ("bug"), could you please create a very simple test case somehow?

@mgoncerz
Copy link
Author

I don't know if it helps, but I may add that registering output in the MCStack works perfectly fine under Geant4, because I've used it previously to save signal tracks from the generator.

@karabowi
Copy link
Collaborator

karabowi commented May 20, 2022

IMHO the problem lies inside the VMC.
We initialize the generator in the FairMCApplication::AddIons() function,
and the sink is initialized in the FairMCApplication::InitGeometry() function.
Both of the functions are called from within the VMC, and it seems the order of calling these functions are different
in case of Geant3 and Geant4.

It wouldn't be the first case that the different VMC engines behave differently, you are just the first person to register output branch in the generator initialization.

I will communicate with the VMC expert to confirm my conclusion and see if there's any solution to the problem.

@mgoncerz : meanwhile, can you create a workaround function in your generator with FairRootManager->RegisterAny() and call it in your macro directly sometime between new FairRunSim and run->Init()?

I am sitting in the meeting and coding currently is tough.

@karabowi
Copy link
Collaborator

@fuhlig1 thanks for tip!

I have simply moved the RegisterAny to the constructor of the generator and it works like dandy also in Geant4.
But have to remember it's a temporary hack, that could lead to race condition, should the user create the generator before FairRunSim.

@mgoncerz
Copy link
Author

mgoncerz commented May 20, 2022

I can also confirm that moving registration to the constructor works flawlessly. Must be an issue of where the Init is called within the VMC.

I am sitting in the meeting and coding currently is tough.

No worries! :)
I'm perfectly fine with workarounds. I'll update my code when it gets fixed in future releases.

@ChristianTackeGSI
Copy link
Member

@karabowi Can you try #1186 and see, if it generates a warning LOG entry for you?

@karabowi
Copy link
Collaborator

Works as intended. I would like to add the topic to our discussion list on Monday.

[INFO] FairBoxGenerator: particle with PDG =211 Found
[WARN] FairRootFileSink::RegisterAny called for elee after FairRootFileSink::CreatePersistentBranchesAny has ran
Converting VMC cuts in regions

@ChristianTackeGSI
Copy link
Member

Works as intended. […]

[INFO] FairBoxGenerator: particle with PDG =211 Found
[WARN] FairRootFileSink::RegisterAny called for elee after FairRootFileSink::CreatePersistentBranchesAny has ran
Converting VMC cuts in regions

Cool. You might consider merging it? At least as a quick warning to the user, that things might go wrong?

I would like to add the topic to our discussion list on Monday.

Just added it to the list. (You could have done that as well.)

@dennisklein dennisklein added the need-triage Missing milestone assignment label Nov 11, 2022
@karabowi
Copy link
Collaborator

I have contacted Ivana and the difference between G3 and G4 is part of the design. Let me quote:

Yes, it is intended. The Geant4 initialization is performed via calling user classes, which are implemented in our case in Geant4 VMC, where we perform the calls to VMC Application functions. This guarantees the correct behaviour in both sequential and MT modes.

So closing the issue, as there will be no developments in the near future.

karabowi added a commit to karabowi/FairRoot that referenced this issue Jul 8, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from the latter of
`InitGeometry()` and `AddIons()`.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
karabowi added a commit to karabowi/FairRoot that referenced this issue Jul 10, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from the latter of
`InitGeometry()` and `AddIons()`.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
karabowi added a commit to karabowi/FairRoot that referenced this issue Jul 23, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from `InitMC()`
and from `InitOnWorker()` for TGeant4 MT mode.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
karabowi added a commit to karabowi/FairRoot that referenced this issue Aug 28, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from `InitMC()`
and from `InitOnWorker()` for TGeant4 MT mode.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
karabowi added a commit to karabowi/FairRoot that referenced this issue Sep 10, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from `InitMC()`
and from `InitOnWorker()` for TGeant4 MT mode.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
karabowi added a commit to karabowi/FairRoot that referenced this issue Sep 13, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from `InitMC()`
and from `InitOnWorker()` for TGeant4 MT mode.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
karabowi added a commit to karabowi/FairRoot that referenced this issue Sep 16, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from `InitMC()`
and from `InitOnWorker()` for TGeant4 MT mode.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues FairRootGroup#1183 and FairRootGroup#1567.
ChristianTackeGSI pushed a commit that referenced this issue Sep 16, 2024
Introduced new function `FairMCApplication::InitFinalizer()`
which initializes event generator, tasks, and triggers
`FairRootManager::WriteFolder()` function.
This new function is called from `InitMC()`
and from `InitOnWorker()` for TGeant4 MT mode.

Previously the funcionality of `InitFinalizer` was
split between `InitGeometry()` and `AddIons()`, which caused
problems because
TGeant3 calls `AddIons()` first, `InitGeometry()` second,
TGeant4 calls `InitGeometry()` first, `AddIons()` second.

Current implementation assures that initialization of
event generators and tasks come before `WriteFolder`.

Fixes issues #1183 and #1567.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-triage Missing milestone assignment
Projects
None yet
Development

No branches or pull requests

4 participants