YAML configuration of reconstruction parameters [online]

The merge request introduces a configuration of the online parameters vs. the reconstruction parameter tag. The tag is defined for a particular runID range. Within the range detector calibrations and alignment matrices must be unchanged (e.g., changing the alignment matrices requires re-generation of the hit-finder and tracking parameters).

EDIT PAL: This MR needs (is built on top of) !2123 (merged)
This merge request is required for !2094.

Conventions:

• all the parameter files, which are required for the reconstruction (event or timeslice), are stored under a particular tag, which is defined in CbmRuntimeDatabase.yaml for a given runID range;

• all the paths are defined relative to ${VMCWORKDIR}/parameters/online directory

• the top directory for reconstruction parameter files and the configuration file is ${VMCWORKDIR}/parameters/online/recoPar

• each file has always the same name (e.g. "StsHitfind.yaml", "TofHitfinerPar.yaml" etc.)

  UPDATE AFTER WORKING ON COMMENTS

• all configuration for cbm::algo (including unpackers, hit-finders, tracking, event-building, event-selection, event-triggering, v0-finder) is defined in a common yaml-config (the name is optional);

• paths to the parameters of the algorithms are defined in node "parFiles" of the main config relative to the path the main config;

• cbmreco accepts a path to the main config, not to a common parameters directory for online (option -p is re-used here);

• online running of cbmreco does not require any information on the runID to define parameters, offline running of the algorithms in cbmreco rely on pre-defined MainConfig.yaml files, stored in cbmroot_parameter repository by path parameters/online/recoPar/<tag>/MainConfig.yaml;

• access to the MainConfig.yaml path by a given run ID is provided by CbmRunDatabaseContainer::GetAlgoMainConfigPath() from C++ code (including ROOT macros) relatively to ${VMCWORKDIR} and by calling $ run_info --run <runID> --algo-config from bash absolutely.

added Online Parameters Reconstruction mCBM labels

requested review from @p.-a.loizeau

assigned to @v.friese

marked this merge request as draft

changed title from online: to Draft: YAML configuration of reconstruction parameters [online]

changed the description

added 1 commit

• d4452839 - online:

Compare with previous version

mentioned in merge request CbmSoft/cbmroot_parameter!245 (merged)

Dear @d.smith, @fweig, @f.uhlig, @v.friese, @p.-a.loizeau, @se.gorbunov, @s.zharko,

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.

added CodeOwners label

The merge-request requires CbmSoft/cbmroot_parameter!245 (merged)

added 1 commit

• 43834e43 - online:

Compare with previous version

marked this merge request as ready

Seems OK for me at a first glance, must confess however that I do not have the time right now to review in details MRs linked to algo/online binary

added 6 commits

• 43834e43...ad593652 - 5 commits from branch computing:master
• 09ab7c57 - online:

Compare with previous version

requested review from @j.decuveland and removed review request for @p.-a.loizeau

added 2 commits

• 08dc3388 - 1 commit from branch computing:master
• 93993444 - online:

Compare with previous version

added 9 commits

• 93993444...a58905f1 - 7 commits from branch computing:master
• 8a3121fc - A compile-time map for a subset of an enumeration
• e92cbc8a - online:

Compare with previous version

mentioned in merge request !2094

added 14 commits

• e92cbc8a...0a87288c - 12 commits from branch computing:master
• b3c9f0c2 - A compile-time map for a subset of an enumeration
• 6657a6e7 - online:

Compare with previous version

added 4 commits

• 6657a6e7...de3fcc76 - 2 commits from branch computing:master
• 267ccaac - A compile-time map for a subset of an enumeration
• d5e96500 - online:

Compare with previous version

I've been thinking about this development, and I have a few reservations. I shared them during our online meeting, but I wanted to reiterate them here since I've been asked for a review.

I don't have any formal objections to this first MR - from a quick review, it looks fine technically. However, my concern with this MR is that I believe the overall parameter handling approach in the online binary is heading in the wrong direction. While these changes don't make it substantially worse than before, they perhaps make it "prettier wrong."

I think storing parameters in .yaml files makes sense. What I find structurally questionable is hardcoding filenames or path components of these files in the code. In my view, there should be a main configuration file passed as a parameter to the program. For an additional indirection layer, this file could then specify the names of (or relative paths to) other .yaml files.

Felix initially violated this principle when he introduced a configuration DIRECTORY as a parameter, where "well known filenames" are searched for. Since then, development has continued further in what I consider a structurally unfavorable direction.

Things go really wrong in my opinion since we essentially built parameter versioning into the online binary. I understand the motivation: we want to test offline with different .tsa files from different runs, and it should "just work." But this approach is based on a fundamentally offline-centered system view. We need to be able to test and develop the online binary offline, but online deployment is the goal of all this work. Online, there aren't different versions - there's only NOW. And exactly this isn't properly supported by this offline system.

I believe we don't need much to get to a fundamentally usable online parameter handling:

1. A directory with .yaml files that are valid NOW

2. A way to modify these (could be a UI for shifters, a Git repo that's automatically checked out there, or initially just a text editor under a shared user account - we already have the latter)

3. Archiving the directory for each run (simple and good would be the ATLAS solution: automatically commit the directory to a Git repository at the beginning of each run and define a tag for the run)

For retrospective work with .tsa files, you'd simply retrieve the corresponding parameters that were current for each run. With Git, this could be more elegant in the future. And if you want to continue offline and pursue hypotheses like "How would it have run online if we had already had the better alignment from my_new_alignment.yaml back then?" - you'd modify the corresponding file in your copy and try it.

What I see here is a mixing/confusion of concepts. A kind of run database was built into the online binary, which doesn't make sense online. This MR now moves this RunDb from the binary partially into specially designed .yaml files. But this doesn't make it better, because it doesn't belong there either.

Just a heads up, I'll be out of the office starting tomorrow and won't be back until Monday, July 28th. To avoid any delays, I'd really appreciate it if someone could take over as a reviewer in my absence. Thanks a bunch!

added 13 commits

• d5e96500...bd8f97bb - 11 commits from branch computing:master
• ecc4913c - A compile-time map for a subset of an enumeration
• bd47ffa1 - online:

Compare with previous version

I would propose a following parameter handling scheme:

• Configuration -- a list of different settings, which can be configured by a user for a particular execution of the cbmreco in a single YAML-file:

  • common properties of cbm::algo: active subsystems (to be removed from cbm::algo::Options), contents of cbm::algo::RecoParams (RecoParams.h, RecoParams.yaml)
  • configuration for different algorithms in a dedicated YAML node (e.g. tracking - ca_params_online.yaml, event building - EventbuildConfig.yaml)
  • paths to different parameter files (e.g., to kf::Setup, hitfinder parameters, readout parameters)

• Parameter files -- individual parameters of the algorithms, which must be provided by experts and should be prepared in advance

@v.friese, @j.decuveland, @p.-a.loizeau, @f.uhlig, @se.gorbunov, what do you think about it?

requested review from @p.-a.loizeau and removed review request for @j.decuveland

changed the description

mentioned in merge request !2097

changed milestone to %JUL25

added 21 commits

• bd47ffa1...ecd83d20 - 18 commits from branch computing:master
• 5ee99009 - A compile-time map for a subset of an enumeration
• 0683a01f - online:
• 4c1887c8 - online: parameter file handling

Compare with previous version

added 33 commits

• 4c1887c8...6064b5ed - 29 commits from branch computing:master
• eb53ed1e - A compile-time map for a subset of an enumeration (+ unit-test)
• ca9f0a9f - online:
• d2c59681 - online: parameter file handling
• caf0c0d6 - updates to run_info service

Compare with previous version

added 1 commit

• 9f35a19a - updates to run_info service

Compare with previous version

added 1 commit

• 848ca5ce - updates to run_info service

Compare with previous version

@p.-a.loizeau Will you have time to review this? If not, please let me know.

added 8 commits

• 848ca5ce...ece334ba - 5 commits from branch computing:master
• a3acd2ab - online:
• 5c949f5e - online: parameter file handling
• 11b49ee0 - updates to run_info service

Compare with previous version

Stopped the pipeline as I rebased before remembering that there is a change of the parameter repository used which is anyway pending.

Sorry for the confusion

added 4 commits

• c7fa52b0 - online:
• 6e36bcf5 - online: parameter file handling
• b1a619b9 - updates to run_info service
• 82d47a06 - parameters hash bump

Compare with previous version

Considering the general comments by @j.decuveland, I think that they are basically accounted for in the updates to this MR.

I must confess I still donot see the full picture of parameter handling, but I am convinced that this MR improves the current situation. I was not able to digest every detail, but I will fo gor merging.

requested review from @v.friese and removed review request for @p.-a.loizeau

approved this merge request

changed the description

added 1 commit

• f98a94f8 - bugfix: proper handling of std::getenv() in CbmRunDatabaseContainer and run_info

Compare with previous version

added 6 commits

• 2fda536a - 1 commit from branch computing:master
• cb89d7d8 - online:
• cd0347e7 - online: parameter file handling
• 1713cfea - updates to run_info service
• c4bf06b5 - parameters hash bump
• bc79e354 - bugfix: proper handling of std::getenv() in CbmRunDatabaseContainer and run_info

Compare with previous version

resolved all threads

mentioned in merge request !2168 (merged)

merged