fix: +1 for L1 latency and trigger class check improved #13682

lietava · 2024-11-12T15:14:43Z

Two issues:
1.)
+1 for L1 input latency.
L1 latency for Trigger Class and Trigger input differs by 1. It is constant
2.) check if class has non zero inputs
if parallel runs then this check produce errors. Therefore class mask for trigger classes for a given run is checked only

github-actions · 2024-11-12T15:15:08Z

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0
async-2024-PbPb-cpass0
async-2024-PbPb-apass1
async-2024-ppRef-apass1

lietava · 2024-11-12T15:21:00Z

async--pbpb-
async--PbPb-

lietava · 2024-11-13T09:51:39Z

Hi @shahor02 , for the labels this is needed for all PbPb reco.

shahor02 · 2024-11-13T11:46:35Z

Hi @lietava , could you please provide more details? Is this only for the pbpb24 or also for pp and pbpb23?
We use L0_L1 also here:

AliceO2/Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

Line 92 in f1eaeb2

    
           int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1 - 1;

Is it ok?

lietava · 2024-11-13T17:06:04Z

Hi @shahor02 , thanks for question. Please, read description - I updated.
Yes, I am aware that L0-L1 latency is used also on the place you pointed out. That one is used only for trigger classes.
I realised that there is difference 1 BC between trigger class latency and input latency - fw feature.
This latency fix concernes only ZDC inputs - we did not use any other L1 inputs, so it is not critical.
For year 2023 the zdc inputs were properly aligned only in the middle of the run - I would not introduce special fix for that.
For 2024 after fix all inputs should be sitting at LM time.
Then other issue: in decodong inputs (shiftInputs) I am doinf check if trigger class has non zero inputs in same BC. However if there is parallel technical run with internal ctp input generaors it produces a lot of errors. Data are ok , just errors are meningless.
So I introduced fix for that. Disadvantege is that I need to access CTPConfig. If that is a problem I remove error check .
I introduced fix only RawDataDecoder - if you approve this also CTFCoder.h has to access CTPConfig - I did not do that.

shahor02

Thanks for explanation, will merge after CI

Detectors/CTP/macro/TestConfig.C

alibuild · 2024-11-16T15:07:23Z

Error while checking build/O2/fullCI for 0de363f at 2024-11-22 09:16:

## sw/BUILD/O2-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'


## sw/BUILD/O2Physics-latest/log
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:


## sw/BUILD/O2-full-system-test-latest/log
Detected critical problem in logfile reco_ASYNC.log
reco_ASYNC.log:[67706:ctp-entropy-decoder]: [08:15:55][ERROR] Exception while running: InputRecord::get: no input with binding ctpconfig found. Available inputs: ctf_CTP, ctfdict_CTP, trigoffset. Rethrowing.
reco_ASYNC.log-[67706:ctp-entropy-decoder]: [08:15:55][FATAL] Unhandled o2::framework::runtime_error reached the top of main of o2-ctf-reader-workflow, device shutting down. Reason: InputRecord::get: no input with binding ctpconfig found. Available inputs: ctf_CTP, ctfdict_CTP, trigoffset
[67706:ctp-entropy-decoder]: [08:15:55][ERROR] Exception while running: InputRecord::get: no input with binding ctpconfig found. Available inputs: ctf_CTP, ctfdict_CTP, trigoffset. Rethrowing.
[67706:ctp-entropy-decoder]: [08:15:55][FATAL] Unhandled o2::framework::runtime_error reached the top of main of o2-ctf-reader-workflow, device shutting down. Reason: InputRecord::get: no input with binding ctpconfig found. Available inputs: ctf_CTP, ctfdict_CTP, trigoffset
[ERROR] Workflow crashed - PID 67706 (ctp-entropy-decoder) did not exit correctly however it's not clear why. Exit code forced to 128.


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
++ GRERR=1
++ [[ 1 == 0 ]]
++ mkdir -p /sw/INSTALLROOT/8d2c25513627b95731c00de6f69ff2fc8b4d21bd/slc8_x86-64/o2checkcode/1.0-local590/etc/modulefiles
++ cat
--

Full log here.

lietava · 2024-11-24T09:59:53Z

HI @shahor02 , I think this went through ?
Or we wait for finishing jira discussion ?

lietava · 2024-11-24T16:00:40Z

Hi @shahor02 , I did changes.
I modified also original name in TriggerOffset which is probably not ok ?
Then I am not sure how TriggerOffset reads database - I do not do it explicily ?

lietava · 2024-11-24T16:02:18Z

@shahor02 : ok I see your answer on jira - going to change

shahor02 · 2024-11-24T16:08:59Z

What you can do is: leave and use these changes but leave also the old L0_L1 data member.

shahor02 · 2024-11-24T16:14:48Z

To load it from the CCDB you should do the same as in the entropy decoders, i.e. add to your raw decoder

AliceO2/Detectors/CTP/workflow/src/EntropyDecoderSpec.cxx

Line 90 in ce63ff8

    
           inputs.emplace_back("trigoffset", "CTP", "Trig_Offset", 0, Lifetime::Condition, ccdbParamSpec("CTP/Config/TriggerOffsets"));

and read the input with mTrigOffsBinding="trigoffset" as in

AliceO2/Detectors/Base/src/CTFCoderBase.cxx

Line 54 in ce63ff8

    
           pc.inputs().get<o2::ctp::TriggerOffsetsParam*>(mTrigOffsBinding); // this is a configurable param

.
That's all.

lietava · 2024-11-27T08:56:48Z

@shahor02 : can you, please, if I understood you correctly ?

shahor02 · 2024-11-27T09:14:08Z

Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

@@ -89,7 +89,7 @@ int RawDataDecoder::addCTPDigit(uint32_t linkCRU, uint32_t orbit, gbtword80_t& d
    }
  } else if (linkCRU == o2::ctp::GBTLinkIDClassRec) {
    int32_t BCShiftCorrection = -o2::ctp::TriggerOffsetsParam::Instance().customOffset[o2::detectors::DetID::CTP];
-    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1 - 1;
+    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1_classes - 1;


You have already L0_L1_classes as 280, do you need to subtract 1?

It was like that before: there was '-1' with 280 always for classes. So I keep it backward compatible.

@lietava what about this?

sorry, what do you mean - I commented above:
It was like that before: there was '-1' with 280 always for classes. So I keep it backward compatible.
And as L0_L1_classes is not in old database onjects - it will be 280 in that case as before (default in old TriggerOffsetParams). For new CCDB we should put also 280. Or we actually do not need new entry as with the current
L0_L1 classes is default of class and L0_L1 inputs is 281.
Or let me know if something not correct.

HI @shahor02 ,
I did validation 559672 and 559243.
Validation means that o2-ctp-reco-workflow --no-lumi --ctpinputs-decoding
gives that same digits as CTF decoder
and both are consistent wirh filling scheme.

@lietava yes, you have ignored the comment above, could you please check?

shahor02 · 2024-11-27T09:18:22Z

Detectors/CTP/workflow/src/RawDecoderSpec.cxx

+    if (mDecodeinputs) {
+      const auto ctpcfg = inputs.get<o2::ctp::CTPConfiguration*>("ctpconfig");
+      if (ctpcfg != nullptr) {
+        mDecoder.setCTPConfig(*ctpcfg);
+      }
+    }


This should be done only once in the beginning of the run processing, right? Why don't you do thin in after https://github.com/AliceO2Group/AliceO2/pull/13682/files#diff-730222aede43b6dd828e45b24b01ba97b3d88fc8c073a79837364b2fb9408b65R237 ?

done, also for EntropyDecoderSpec

lietava · 2024-11-27T17:36:29Z

HI @shahor02 , I did modification for updating ctpconfig only if run changes in both ctp and ctf decoders.

alibuild · 2024-11-27T22:07:20Z

Error while checking build/O2/fullCI for f8c8b97 at 2024-11-27 23:07:

## sw/BUILD/O2-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'

Full log here.

lietava · 2024-11-28T13:39:22Z

HI @shahor02 , please, let me know if I missed something or you have more comments.

lietava · 2024-11-14T09:21:50Z

Detectors/CTP/workflow/src/RawDecoderSpec.cxx

@@ -135,6 +137,11 @@ void RawDecoderSpec::run(framework::ProcessingContext& ctx)
  if (fatal_flag) {
    ret = mDecoder.decodeRawFatal(inputs, filter);
  } else {
+    if (mDecodeinputs) {
+      const auto ctpcfg = inputs.get<o2::ctp::CTPConfiguration*>("ctpconfig");


what will happen if ctpcfg will not be found ?
nullptr is returned ?

Detectors/CTP/macro/TestConfig.C

lietava · 2024-11-27T15:58:31Z

Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

@@ -89,7 +89,7 @@ int RawDataDecoder::addCTPDigit(uint32_t linkCRU, uint32_t orbit, gbtword80_t& d
    }
  } else if (linkCRU == o2::ctp::GBTLinkIDClassRec) {
    int32_t BCShiftCorrection = -o2::ctp::TriggerOffsetsParam::Instance().customOffset[o2::detectors::DetID::CTP];
-    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1 - 1;
+    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1_classes - 1;


It was like that before: there was '-1' with 280 always for classes. So I keep it backward compatible.

lietava · 2024-11-27T17:32:48Z

Detectors/CTP/workflow/src/RawDecoderSpec.cxx

+    if (mDecodeinputs) {
+      const auto ctpcfg = inputs.get<o2::ctp::CTPConfiguration*>("ctpconfig");
+      if (ctpcfg != nullptr) {
+        mDecoder.setCTPConfig(*ctpcfg);
+      }
+    }


done, also for EntropyDecoderSpec

lietava · 2024-11-27T18:32:55Z

Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

@@ -89,7 +89,7 @@ int RawDataDecoder::addCTPDigit(uint32_t linkCRU, uint32_t orbit, gbtword80_t& d
    }
  } else if (linkCRU == o2::ctp::GBTLinkIDClassRec) {
    int32_t BCShiftCorrection = -o2::ctp::TriggerOffsetsParam::Instance().customOffset[o2::detectors::DetID::CTP];
-    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1 - 1;
+    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1_classes - 1;


sorry, what do you mean - I commented above:
It was like that before: there was '-1' with 280 always for classes. So I keep it backward compatible.
And as L0_L1_classes is not in old database onjects - it will be 280 in that case as before (default in old TriggerOffsetParams). For new CCDB we should put also 280. Or we actually do not need new entry as with the current
L0_L1 classes is default of class and L0_L1 inputs is 281.
Or let me know if something not correct.

lietava · 2024-11-28T09:26:38Z

Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

@@ -89,7 +89,7 @@ int RawDataDecoder::addCTPDigit(uint32_t linkCRU, uint32_t orbit, gbtword80_t& d
    }
  } else if (linkCRU == o2::ctp::GBTLinkIDClassRec) {
    int32_t BCShiftCorrection = -o2::ctp::TriggerOffsetsParam::Instance().customOffset[o2::detectors::DetID::CTP];
-    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1 - 1;
+    int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1_classes - 1;


HI @shahor02 ,
I did validation 559672 and 559243.
Validation means that o2-ctp-reco-workflow --no-lumi --ctpinputs-decoding
gives that same digits as CTF decoder
and both are consistent wirh filling scheme.

shahor02 · 2024-11-28T14:19:06Z

Would not it be now 279? L0_L1_classes is 280.

lietava · 2024-11-28T14:32:39Z

ON te current version, i.e. like in branch dev:
in the TriggerOssetsParams there is 280:

AliceO2/DataFormats/Detectors/CTP/include/DataFormatsCTP/TriggerOffsetsParam.h

Line 27 in 74c640e

int64_t L0_L1 = 280;

This was used for classes in ctprawdecoder:

AliceO2/Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

Line 92 in 74c640e

    
           int32_t offset = BCShiftCorrection + o2::ctp::TriggerOffsetsParam::Instance().LM_L0 + o2::ctp::TriggerOffsetsParam::Instance().L0_L1 - 1;

as there was no CCDB reading
In CTF decoder classes are never done. There in:

AliceO2/Detectors/CTP/reconstruction/include/CTPReconstruction/CTFCoder.h

Line 219 in 74c640e

o2::ctp::RawDataDecoder::shiftInputs(digitsMap, digits, mFirstTFOrbit);

281 was used in:

AliceO2/Detectors/CTP/reconstruction/src/RawDataDecoder.cxx

Line 532 in 74c640e

auto L1shift = L0shift + o2::ctp::TriggerOffsetsParam::Instance().L0_L1;

because CCDB was read

lietava · 2024-11-28T14:34:57Z

I believe my new code is doing exactly the same and it also does correct ctpinputs decodong when done
in ctprawdecoder:
workflow --shm-segment-size 12000000000 --no-lumi --ctpinputs-decoding -b
which was not case before.

fix: +1 for L1 latency

7c46d60

alibuild mentioned this pull request Nov 12, 2024

Please consider the following formatting changes to #13682 lietava/AliceO2#38

Closed

fix: +1 for L1 latency

f1eaeb2

lietava marked this pull request as ready for review November 12, 2024 15:21

lietava requested a review from a team as a code owner November 12, 2024 15:21

lietava added 2 commits November 13, 2024 17:47

dev: decoder: checking only trigger class bits which belongs to run

e4efba2

clang

5b3513d

lietava changed the title ~~fix: +1 for L1 latency~~ fix: +1 for L1 latency and trigger class check improved Nov 13, 2024

shahor02 previously approved these changes Nov 13, 2024

View reviewed changes

shahor02 reviewed Nov 13, 2024

View reviewed changes

Detectors/CTP/macro/TestConfig.C Outdated Show resolved Hide resolved

lietava added 2 commits November 16, 2024 12:34

dev: ctp config added also to CTF decoder

ba4bea3

clang

0de363f

lietava dismissed shahor02’s stale review via 0de363f November 16, 2024 11:35

lietava added 4 commits November 22, 2024 18:51

fix: getting ctpconfig

08537a4

clang

d2f967d

fix: removing std:;cout

6709adb

clang

e72be89

lietava added 3 commits November 24, 2024 15:37

removing macro's modification to have cleaner PR

e88df9a

dec: two latency vars, rew-decoder accessing ccdb

a48ba08

clang

5f6de5a

lietava added 4 commits November 24, 2024 17:35

TriggerParams old variable same name

61219d7

clang

4fb5a0f

dev: reading of CCDB offset paraams in rawdatadecoder

0c89e30

clang

c425e86

shahor02 reviewed Nov 27, 2024

View reviewed changes

dev: config done only if run changed

f8c8b97

lietava commented Nov 28, 2024

View reviewed changes

shahor02 merged commit 66b81d8 into AliceO2Group:dev Nov 28, 2024
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: +1 for L1 latency and trigger class check improved #13682

fix: +1 for L1 latency and trigger class check improved #13682

lietava commented Nov 12, 2024 •

edited

Loading

github-actions bot commented Nov 12, 2024

lietava commented Nov 12, 2024

lietava commented Nov 13, 2024

shahor02 commented Nov 13, 2024

lietava commented Nov 13, 2024

shahor02 left a comment

alibuild commented Nov 16, 2024 •

edited

Loading

lietava commented Nov 24, 2024

lietava commented Nov 24, 2024

lietava commented Nov 24, 2024

shahor02 commented Nov 24, 2024

shahor02 commented Nov 24, 2024

lietava commented Nov 27, 2024

shahor02 Nov 27, 2024

lietava Nov 27, 2024

shahor02 Nov 27, 2024

lietava Nov 27, 2024

lietava Nov 28, 2024

shahor02 Nov 28, 2024

shahor02 Nov 27, 2024

lietava Nov 27, 2024

lietava commented Nov 27, 2024

alibuild commented Nov 27, 2024

lietava commented Nov 28, 2024

lietava Nov 14, 2024

shahor02 Nov 28, 2024

lietava Nov 27, 2024

lietava Nov 27, 2024

lietava Nov 27, 2024

lietava Nov 28, 2024

shahor02 commented Nov 28, 2024

lietava commented Nov 28, 2024

lietava commented Nov 28, 2024

fix: +1 for L1 latency and trigger class check improved #13682

fix: +1 for L1 latency and trigger class check improved #13682

Conversation

lietava commented Nov 12, 2024 • edited Loading

github-actions bot commented Nov 12, 2024

lietava commented Nov 12, 2024

lietava commented Nov 13, 2024

shahor02 commented Nov 13, 2024

lietava commented Nov 13, 2024

shahor02 left a comment

Choose a reason for hiding this comment

alibuild commented Nov 16, 2024 • edited Loading

lietava commented Nov 24, 2024

lietava commented Nov 24, 2024

lietava commented Nov 24, 2024

shahor02 commented Nov 24, 2024

shahor02 commented Nov 24, 2024

lietava commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lietava commented Nov 27, 2024

alibuild commented Nov 27, 2024

lietava commented Nov 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahor02 commented Nov 28, 2024

lietava commented Nov 28, 2024

lietava commented Nov 28, 2024

lietava commented Nov 12, 2024 •

edited

Loading

alibuild commented Nov 16, 2024 •

edited

Loading