Large commit for Rivet integration

[cholmcc]

This commit imports the O2Rivet project into O2Physics.

This project aims to enable running Rivet analyses on existing and future simulation productions, locally, on CVMFS, on Grid, and with HyperLoop.

The main component is the DPL o2-analysis-mm-rivet. This reads in MC AODs (via f.ex. o2-aod-mc-producer-workflow or o2-aod-producer-workflow), converts the events to HepMC events, and then runs Rivet analyses on these. The output is an o2::rivet::RivetAOs object stored in the generated AnalysisResults.root file.

The tool supports partial runs (e.g., parallel processing on Grid), and merging, including the Rivet::Analysis::finalize step, via regular ROOT merging.

The tool also supports compiling analyses on the fly. This is useful in Grid or similar settings, and for developing and testing analyses not yet accepted into an official repository.

Examples of using the tool are given in PWGMM/Rivet/doc along with some other documentation.

Compilation of the tool is contingent on the presence of Rivet (and its requirements YODA, FastJetContrib, and HepMC3) on the target system. The feature is marked as recommended via optional, but recommended, build dependencies on Rivet and YODA.

Getting this PR into $0^2\mathrm{Physics}$ is one of final steps of fully integrating Rivet into the $O^2$ framework. Other than this PR, we also need to have

• Rivet properly deployed in the various compute platforms,
• $O^2\mathrm{Physics}$ deployed with Rivet support built in

WRT the compute platforms. The above cited PR against alidist will take care of CFMFS and the like (e.g., LXPLUS), but it is still a bit of an open question as for HyperLoop. Hopefully some experts can help out with that.

Yours,

Christian

[cholmcc]

Formatting checker complains

The following error is for file PWGMM/Rivet/doc/analyses/ALICE_YYYY_I1234567.yoda, line 7, column 7:
Error: Indent code using spaces instead of tabs.

However, the YODA file is not code but data. Can someone make an exception for YODA files please? Thanks.

[cholmcc]

 PWGMM/Rivet/DataModel/RivetAOs.cxx:71:  Use operator ! instead of not  [readability/alt_tokens] [2]

Really? Do people really think that ! is easier to read than not? I think the authors of Python would strongly disagree.

Could we take that check out? I mean, can we not have a bit of freedom in how we write our code? Thanks.

[cholmcc]

chktex: WARNING -- Could not find global resource file.
  Warning 36 in PWGMM/Rivet/doc/presentation/intro.tex line 303: You should put a space in front of parenthesis.
          \tikz[baseline={ (0,-.1)}]{\draw[con,dashed](0,0) -- (.4,0);} not  
  
  Warning 8 in PWGMM/Rivet/doc/presentation/intro.tex line 303: Wrong length of dash may have been used.
          \tikz[baseline={ (0,-.1)}]{\draw[con,dashed](0,0) -- (.4,0);} not  

Both of these errors are plain wrong. I might be able to fix them, but they are deadly wrong. -- is used in Tik_z_ to draw a line, and there's no need to have an additional space in front of the parenthesis. Please fix in checker. Thanks.

[cholmcc]

PWGMM/Rivet/DataModel/RivetAOs.cxx:19:  Found C system header after other header. Should be: RivetAOs.h, c system, c++ system, other.  [build/include_order] [4]

Linter seems to think that <YODA/IO.h> is a C system header - that's wrong, and illustrates that the rational for the check is flawed at best, erroneous at worst.

[cholmcc]

flake8 seems to think there's a comment in this line

        dot = self.make_dot(ev, no)

which there obviously isn't - disable flake8 altogether.

[cholmcc]

Perhaps someone need to approve so that the CI's can continue. Thanks.

[aalkin]

No comment on the code, but adding an explicit dependency of O2Physics on Rivet and co. is a no go. Even more, does it even use anything from O2Physics? It seems to depend on O2 only. Please make a case at WP4+14 meeting first, so that the proper place for this toolset can be decided.

[cholmcc]

No comment on the code, but adding an explicit dependency of O2Physics on Rivet and co. is a no go.

It is not a dependency but a recommendation. That is, you can configure $O^{2}\mathrm{Physics}$ without Rivet, but you need not. BTW, in terms of optional - relatively lightweight - dependencies it is only Rivet and YODA that are added. See here.

The rest, fastjet(contrib) and HepMC3 are already (indirect) dependencies via $O^{2}$.

The question is, where, if any, should the Rivet optional dependency become mandatory. I would argue that for deployment of $O^2$ / $O^2\mathrm{Physics}$ on CVMFS (Grid), and HyperLoop it should be mandatory as it will give collaborators easy access to large data sets to run their Rivet analyses on - for analysis validation but, perhaps more importantly, consistent comparisons to models - which is the whole point of the exercise.

Even more, does it even use anything from O2Physics?

As much as any other analysis uses anything from $O^2\mathrm{Physics}$. The code analyses only simulated data, which makes its dependencies small.

Please make a case at WP4+14 meeting first, so that the proper place for this toolset can be decided.

Well, I've been trying to get people - including yourself - to chime in for some time now, but no-one has been willing to come up with decision or even a recommendation. I did ping PWGMM conveners about a 2 weeks ago to see if they had any thoughts - but crickets I'm afraid. You and I also had a long conversation on MatterMost but in the end you never came back to me on this and other issues.

As far as I understand, the issue is that HyperLoop only deals with two packages $O^2$ and $O^2\mathrm{Physics}$, and adding a third package $O^2\mathrm{Rivet}$ would be a problem. That means, that if the collaboration is serious about Rivet, then this code would need to go into either $O^2$ or $O^2\mathrm{Physics}$. $O^2$ maintainers (Sandro) seemed very much opposed to it, and I do agree that it fits better with $O^2\mathrm{Physics}$ as the code is processing $\mathrm{AO}^2\mathrm{D}$ data.

That said, I think the most important thing is that the code is readily available to users on CVMFS and HyperLoop. Which code base that is, $O^2$, $O^2\mathrm{Physics}$, or as a third $O^2\mathrm{Rivet}$ package is in my mind less important.

But I would love to hear yours and others thoughts on this. Where do you see this code going? Of course, we have to keep in mind the end-goal here: To facilitate collaborators to make their Rivetisation of their analyses and make clear and consistent comparisons of data to models. Let us get the ball rolling on that.

Perhaps one thing to do is to allow the CIs to run on this PR, and concurrently have a discussion on where this code should go. I think we need to bring relevant parties from CVMFS and HyperLoop too.

@PWGMM conveners (@coppedis, @sustripathy) and PWGMM/MC coordinators (@maireiphc, @jackal1-66): Perhaps you guys could chime in and help drive the discussion? It is after all in your purview.

Looking forward to a constructive discussion on this.

Thanks.

Yours,

Christian

[aalkin]

This is not the place to have these discussions. Neither O2 or O2Physics is a good place for this, for many reasons. However as a standalone package it can be easily used to run on the grid. Alihyperloop is very specifically for analysis only and will not be able to handle this without additional developments. Please, start with the clear proposal to WP4+14, on how and when this is supposed to be run. It is a good idea to be able to run Rivet analyses on existing productions, however the procedure needs to be clearly defined.

[cholmcc]

So Anton and @pzhristov @ktf , can we set up something in the WPs ASAP? Thanks.

Just one point, a Rivet analysis is an analysis as it will run on (MC) AODs - at least, that's the goal - so as to make consistent comparisons between data and models.

Yours,

Christian

[cholmcc]

Neither O2 or O2Physics is a good place for this, for many reasons.

Could you please elaborate on that? It would be good to know what you see as the problems. Thanks.

Yours,

Christian

[pzhristov]

So Anton and @pzhristov @ktf , can we set up something in the WPs ASAP? Thanks.

Just one point, a Rivet analysis is an analysis as it will run on (MC) AODs - at least, that's the goal - so as to make consistent comparisons between data and models.

Yours,

Christian

Hi @cholmcc We have the WP4/WP14 today at 9:30, or next Monday at 9:30. Probably it is too short notice for the meeting today, but we can at least start the discussion. Please let me know if you are able to join.

[cholmcc]

Hi @cholmcc We have the WP4/WP14 today at 9:30, or next Monday at 9:30. Probably it is too short notice for the meeting today, but we can at least start the discussion. Please let me know if you are able to join.

Great, so please schedule a slot next Monday the 29th of January.

@coppedis, @sustripathy, @maireiphc, and @jackal1-66 can you please be available for that discussion too. Thanks.

@aalkin I would still very much like to hear your reasons why you think the Rivet integration does not fit within $O^2$ nor $O^2\mathrm{Physics}$. If not here, then through some other means of communication (email, MatterMost).

Thanks.

Yours,

Christian

[pzhristov]

So Anton and @pzhristov @ktf , can we set up something in the WPs ASAP? Thanks.

Just one point, a Rivet analysis is an analysis as it will run on (MC) AODs - at least, that's the goal - so as to make consistent comparisons between data and models.

Yours,

Christian

Hi @cholmcc,
I added your presentation to the WP4/WP14 meeting next week:
https://indico.cern.ch/event/1373002/
Cheers,
Peter

[jackal1-66]

@cholmcc I will join the meeting and the discussion next Monday. Please @aalkin add me on the discussion of the Rivet integration in case it moves via email, since I'm afraid I don't see a reason as well why adding Rivet to O2Physics could cause any harm to the package itself, apart from increasing the analysis tools of the community. We discussed in the past about a possible O2Rivet additional package during another meeting, but from what I understood, that would be challenging to import on hyperloop, which is one of the main targets here. Regarding resources used, in the end, they shouldn't be any higher than a normal analysis.

[cholmcc]

@pzhristov Thanks.

@jackal1-66 Great - see you there.

[jgrosseo]

Thanks @aalkin, @cholmcc and @jackal1-66. I think we can discuss the raised items next Monday. There is no need to do this in parallel in a separate channel or on this PR. In particular because it seems that some of the arguments are driven by seemingly wrong assumptions about the Hyperloop integration.

Please consider that many of the core O2, O2Physics and Hyperloop developers see this package for the first time. Regardless if this is an oversight or not, comments and suggestions are normal. We should not expect that we can resolve everything Monday but see this as a good kick-of the discussion to find the best possible solution.

[cholmcc]

@jgrosseo Could you please elaborate on "some of the arguments are driven by seemingly wrong assumptions about the Hyperloop integration". I would like to understand this before giving a presentation. If not here, then by email or MatterMost (links above). Thanks.

WRT "Please consider that many of the core O2, O2Physics and Hyperloop developers see this package for the first time" - that's not entirely true @aalkin saw it first in September, @sawenzel, @ktf. @TimoWilken around the same time, @pzhristov has also seen it relatively recently, and I think you yourself @jgrosseo has also been party to earlier discussions where $O^2\mathrm{Rivet}$ was mentioned.

For those of you who need a bit more information, please see this presentation. Please note that the information in that presentation is slightly out of date - a more up-to-date presentation can be found in this PR as $\mathrm{\LaTeX}$ sources.

@pzhristov Could you change the contributor on next weeks meeting page to be me - cholm or perhaps [email protected] - I do not have submission access at the moment. Thanks.

Yours,

Christian

[github-actions]

This PR has not been updated in the last 30 days. Is it still needed? Unless further action is taken, it will be closed in 5 days.

[cholmcc]

New merged commit.

This allows multiple HyperLoop wagons to be merged into one.

Some comments

• Depends on the this O2 pull request - not for compilation, but execution
• Merging several wagons only works iff
  • Each wagon has the option --workflow-suffix XX . Note that this option must be given early, if not first, on the command-line
  • The wagons configuration (options) must be specified by a JSON configuration file - e.g., --configuration json://foobar.json
• If the merging code (MmRivet::absorb_or_die) only finds one wagon, then nothing is done
• If more than one wagon is found, then the first (in terms of lexical sorting of the wagon suffixes) will be active, and the remaining will make themselves catatonic (alive, but do nothing). Wagon disables itself by clearing the list of Rivet Analysis to execute
• The first wagon then inspects the configuration of other wagons. This fails if
  • Another wagon specifies a cross-section that is incompatible
  • sets the merge-equivalence to a different value
  • sets the HepMC event recentering to a different value
  • sets the HepMC generator-only flag to a different value
• Otherwise, the various configuration options are merged into the first wagon.

The recent update also does a few other simple fixes.

[vkucera]

Add the mandatory file documentation.

[cholmcc]

Can you please point to some document that lays out what is mandatory and what isn't? Thanks.

[vkucera]

https://github.com/AliceO2Group/CodingGuidelines

[cholmcc]

Clearly, the Comment Guidelines are not being followed in ALICE in general. If that was the case, then there would be a lot more documentation comments in the code, so this seems more like a recommendation than hard and fast requirements.

Also, it seems like the linters do not necessarily follow the guidelines (max 120 characters per line - which really should be at most 80 characters) or require more (no where, as far as I can tell, are not deprecated over ! f.ex.). I think there should be consistency between the guidelines and the linters, etc. How else are anyone expected to know what to do?

[jgrosseo]

Will be undrafted once the discussions are complete.

[cholmcc]

Please re-open this MR. Thanks.

[github-actions]

This PR has not been updated in the last 30 days. Is it still needed? Unless further action is taken, it will be closed in 5 days.

[jackal1-66]

Additional comments will be provided in the next days.