FairMCEventHeader should be a derived class of FairEventHeader

[YanzhaoW]

Hi,

Is your feature request related to a problem? Please describe.

When I analysis the simulation data using FairRunAna generated from the FairRunSim, the event header cannot be filled during the analysis and written to the output file, because the FairEventHeader used by FairRunAna is incompatible with FairMCEventHeader used by FairRunSim

Describe the solution you'd like

A simple solution would be making FairMCEventHeader as a dervied class from FairEventHeader. With this, FairMCEventHeader can be correctly handled in FairRunAna as well.

[karabowi]

Dear @YanzhaoW , FairMCEventHeader and FairEventHeader are two separate and different entities.
Just like FairMCPoint and FairHit are.

FairMCEventHeader(and its possible derivatives) is created during MC simulation,
just as FairEventHeader is created during the analysis, sometimes even during analysis of simulated data.
In such cases, some of the information from FairMCEventHeader may be used to create appropriate FairEventHeader.

Should you have any problem, refer to FairRoot examples in examples/advanced/ .

[YanzhaoW]

Dear @karabowi

Thanks very much for your reply. 😄

FairMCEventHeader and FairEventHeader are two separate and different entities. Just like FairMCPoint and FairHit are.

Yes, I fully understand they are different entities, represented by different classes. But we do have the situation where we use the output data from the FaiRunSim as the input of FairRunAna. This implies FairRunAna should handle the output event header from the FairRunSim. But the way how FairRunAna handles the propagation of the event header is only limited by classes that inherit the FairEventHeader class. This can be seen here:

FairRoot/fairroot/base/steer/FairRun.cxx

Lines 242 to 250 in d6b91e1

	void FairRun::FillEventHeader()
	{
	if (!fSource) {
	LOG(warning) << "FairRun::FillEventHeader: No Source!";
	return;
	}
	fSource->FillEventHeader(fEvtHeader.get());
	}

Due to this limitation, the MCEventHeader is not handled at all by the fSource and users have to create a specific task just to propagate the MCEventHeader.

An ugly solution is to add another function, like FillMCEventHeader() and call it every time when there is a call of FillEventHeader().

A better and less intrusive solution is what I have suggested above: make FairMCEventHeader as a derived class of FairEventHeader and nothing else. Then fSource, which is usually implemented by users, will handle the MCEventHeader from the user side. This is, IMO, more consistent.

[karabowi]

Dear all,

would you be satisfied with the following redesign:

1. Implementation of FairEventHeaderPropagator class, which would look like:

class FairEventHeaderPropagator {
 public:
   virtual ~FairEventHeaderPropagator() = default;
   virtual void Propagate(const FairMCEventHeader* inputHeader, FairEventHeader* outputHeader) const {
        outputHeader->SetRunId(inputHeader->GetRunId());
        outputHeader->SetMCEntryNumber(inputHeader->GetMCEntryNumber());
  }
   virtual void Propagate(const FairEventHeader* inputHeader, FairEventHeader* outputHeader) const {
        outputHeader->SetRunId(inputHeader->GetRunID());
        outputHeader->SetMCEntryNumber(inputHeader->GetEventID());
  }
};

2. For example FairFileSource::FillEventHeader() would thus take form of:

void Fair*Source::FillEventHeader(FairEventHeader* feh) {
    if (fEvtHeader) { 
        fPropagator->Propagate(fEvtHeader, feh);
    } 
    else if (fMCEvtHeader) {
        fPropagator->Propagate(fMCEvtHeader, feh);
    } else {
        LOG(fatal) << "No EventHeader information to propagate";
    }
    feh->SetEventTime(GetEventTime());
    feh->SetInputFileId(0);
}

3. With this you would still need to write a class, but it would be a shortee:

class MyCustomPropagator : public FairEventHeaderPropagator {
public:
  void Propagate(const FairMCEventHeader* inputHeader, FairEventHeader* outputHeader) const override {
    // Implement custom logic to copy/transform data from inputHeader to outputHeader
   outputHeader->SetValue(mcHeader->GetValue());
  }

  void Propagate(const FairEventHeader* inputHeader, FairEventHeader* outputHeader) const override {
    // Implement custom logic to copy/transform data from inputHeader to outputHeader
    outputHeader->SetValue(mcHeader->GetValue());
  }
};

What do you think about this?

[YanzhaoW]

Hi,

If I understand it correctly, the problem is still there. It still doesn't write ("propagate") the simulation event header to the output data. The written header is FairEventHeader, instead of FairMCEventHeader.

[karabowi]

I do not see it as a problem.

I have a problem in the fact that the analysis FairEventHeader should base on the FairMCEventHeader, the simulation event header.
I think it would not be wise to change such drastically the inheritance structure after 20+ years of rather successful working.

Should you need any information from the FairMCEventHeader in the later stages of your analysis, you can always use the friend mechanism, and AddFriend(simulationData) to your input.

[YanzhaoW]

I have a problem in the fact that the analysis FairEventHeader should base on the FairMCEventHeader, the simulation event header.

I think there is a misunderstanding here. I meant FairMCEventHeader should base on FairEventHeader, something like:

class FairMCEventHeader : public FairEventHeader { ... };

[karabowi]

You are correct. Sorry for the misunderstanding. Let me think about this.

[karabowi]

Dear @YanzhaoW ,

After discussing with @fuhlig1, we both oppose the proposed change.

Current Design:

• FairMCEventHeader and FairEventHeader are designed to store different types of data: one for simulation and the other for analysis.
• While you might find it useful to have FairMCEventHeader derived from FairEventHeader, other experiments manage without this feature.
• Implementing this change could negatively impact data access for other experiments, such as ALICE or CBM.

Future Plans:

• We are planning to adopt the new ROOT IO: RNTuple .
• RNTuple is unlikely to support elements like TString or TClonesArray.
• This means we will need to modify our data structures and possibly re-evaluate the event header classes in the future.

[YanzhaoW]

Dear @karabowi

Thanks for your reply.

I would like to restate the reason why FairMCEventHeader should be derived from FairEventHeader:

There are many situations where users need to use the output data from the FairRunSim as the input data of FairRunAna. But these two have incompatible event headers: FairRunAna using FairEventHeader and FairRunSim using FairMCEventHeader. Due to this incompatibility, the output data file of the FairRunAna would have two event headers, one for FairEventHeader, which is useless, and another for FairMCEventHeader, which is useful but not handled by the run. It is also very confusing to have two event headers in the data files, one of which is totally useless.

FairMCEventHeader and FairEventHeader are designed to store different types of data: one for simulation and the other for analysis.

Like I have stated above, the problem arises when one needs to analyze the simulation data.

By simply making FairMCEventHeader derived from FairEventHeader, this incompatibility can be eliminated and the final data file would have just one single event header, which is both useful and handled correctly by the run.

Unless it's not recommended to use output data file from FairRunSim as the input data file of FairRunAna?

Implementing this change could negatively impact data access for other experiments, such as ALICE or CBM.

I don't quite understand the possibly negatively impacts for other experiments. In R3B experiment, for a long time, we are using a different event header R3BEventHeader, which likes FairMCEventHeader has extra data members but is derived from FairEventHeader. The only thing we need to change is set the event header to be R3BEventHeader and nothing else:

run->SetEventHeader(std::make_unique<R3BEventHeader>());

With this, FairRun could automatically read R3BEventHeader from the input data and propagate it to the output files, where only single event header exists for each event, instead of two (R3BEventHeader and FairEventHeader)

Future Plans:
We are planning to adopt the new ROOT IO: RNTuple .
RNTuple is unlikely to support elements like TString or TClonesArray.
This means we will need to modify our data structures and possibly re-evaluate the event header classes in the future.

I'm very glad to see this is a part of the future plan. Currently writing data with STL types is using some weird type_id stuff. Changing it to RNTuple would be much cleaner.