Draft: Add histograms for trail and bunch, trail+branch, isolated

This is to add the histograms after the implementation of the fake TAE. Basically the occupancy as a function of the TAE index is not considered anymore. Now there are separated histograms of occupancy vs TAE index separately for windows around isolated bunch, trail bunch, lead bunch and merged trail+lead. Analogous histograms are added as a function of the bitID

requested review from @jonrob

assigned to @fbetti

added 1 commit

• 6e7d47c6 - fix formatting

Compare with previous version

added 1 commit

• 73e87ef1 - Fixed formatting

Compare with previous version

added 1 commit

• 8e72eb0c - Fixed formatting

Compare with previous version

marked this merge request as ready

marked this merge request as draft

added 1 commit

• 2b0bc512 - restore file before formatting, otherwise it does not compile at the pit

Compare with previous version

added 1 commit

• 19735333 - Fixed formatting

Compare with previous version

marked this merge request as ready

Hi @jonrob , the MR is ready. I tested it at the pit and it works.

I do not like all the code like

eventType & static_cast<std::uint16_t>( 1 << 8 )

It is, in my opinion the wrong place to put code like this into the RICH monitor.

These are questions which the ODIN object should be able to answer, via access methods, in a more direct and obviously-named way.

In fact, the ODIN already has something like this

https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Event/DAQEvent/include/Event/ODIN.h#L362

  [[nodiscard]] constexpr bool isFlagging() const {
        return eventType() & static_cast<std::uint16_t>( EventTypes::HltFlaggingMode );
  }

The class enum already has the addition types defined

https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Event/DAQEvent/include/Event/ODIN.h#L121

So. What I think should happen is you should open a MR against the ODIN, where you propose adding new helper methods, like the one above, for the various bits in the EventType you wish to check. Then, the RICH code should just use these accessors, rather than manipulating the EventType directly itself as you do here.

I agree with you, but @gcavalle told me that this is quite urgent, so for the moment we should do in the "wrong" way. Adding @rmatev in case he wants to comment

I do not mind checking out the changes here into the build I use for the monitoring at the pit, but I do not what to merge this to master until it has been updated to use the appropriate accessors.

The build at the pit has been updated to use the changes here, as a temporary branch. The monitor will pick them up whenever it next restarts.

I will though in the meantime mark this back as Draft, until the appropriate ODIN accessors have been added and this has been updated to use them.

Thanks a lot

@jonrob please check LHCb!4192 (merged)

@fbetti Please check the MR above. Can you please check to see if using it allows you to simplify the changes here ?

marked this merge request as draft

@fbetti can you please comment here ?

Please note, the changes in place at the pit are purely temporary, and liable to go missing when I next update the stack there. The changes here must be in master if you want them to persist going forward, which requires making the changes as discussed above.

Hi @jonrob , really sorry if I did not reply, I am back from holidays and trying to catch up with higher priority tasks. When are you going to update the stack? If it is today-tomorrow, I will try to do it asap

I would like to do it before we get beam back. Lets say next week sometime so if you can get this addressed this week that would be ideal.

Sorry, I had no time this week. I plan to do it tomorrow (Monday). BTW, the mentioned MR (LHCb!4192 (merged)) is a Draft, how should I proceed?

added 54 commits

• 19735333...574317e8 - 44 earlier commits
• ddd7165b - Do not run calibration in stop() during QMT tests as these are ignored in ref comparisons
• 2eae643e - Update test ref logs
• 7b12c9fd - Update References for: LHCb!4248 (merged) based on lhcb-master-mr/8920
• 14e21897 - Fix el9 refs following scheduler changes
• b8e7bde0 - Truncate fit result printout in RefIndexCalib
• 783d2359 - Fix CI job for formatting
• 735619e3 - Apply LHCb formatting
• f369bc16 - Fix errors caused by DD4Hep future upgrades refactorFix morst errors by adding new geometry
• 4c5e02b1 - Update References for: Detector!425 (merged) based on lhcb-master-mr/9351
• 7bcf35a5 - add histogram TAE which includes iso, lead and trail

Compare with previous version

added 1 commit

• 82070a2b - Fixed formatting

Compare with previous version

I close the MR because in 2024 it has been replaced by the new superTAE (see !320 (merged))

closed