Skip to content

[EMCAL-539,EMCAL-696,EMCAL-697] First version of the L0 trigger imple…#12362

Merged
mfasDa merged 12 commits intoAliceO2Group:devfrom
siragoni:EMCAL-539
Feb 5, 2024
Merged

[EMCAL-539,EMCAL-696,EMCAL-697] First version of the L0 trigger imple…#12362
mfasDa merged 12 commits intoAliceO2Group:devfrom
siragoni:EMCAL-539

Conversation

@siragoni
Copy link
Contributor

…mentation

Added:

  • TRU time response
  • Pileup simulation at trigger level
  • Peak finder and L0 algorithm

Simulation not yet implemented in the EMCALDigitizerSPEC

…mentation

Added:
- TRU time response
- Pileup simulation at trigger level
- Peak finder and L0 algorithm

Simulation not yet implemented in the EMCALDigitizerSPEC
Copy link
Collaborator

@mfasDa mfasDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment one small request from my side

@mfasDa mfasDa self-requested a review December 8, 2023 09:25
Copy link
Collaborator

@mfasDa mfasDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First set of change requests. There are two things that are different between trigger and FEE, and because the code is adapted from FEE it is consequently wrong:

a) Phase: FEE has a 10 MHz clock, while the LHC has a 40 MHz clock, consequently the deposited charge per sample depends on the relative BC between LHC and ALTRO clock
b) Noise: FEE has 2 digitizers, with different noise, and we decide only at reconstruction level based on the energy of the full raw fit which of the two to take. Consequently we must simulate for each digit both high and low gain noise. The trigger system has only one digitizer and consequently only one noise value, which is a lot easier to handle.

Copy link
Collaborator

@mfasDa mfasDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next round of change requests. Will still need to go through in more details through the LZERO for the next round. For the moment please focus on those changes.

Copy link
Collaborator

@mfasDa mfasDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite some adjustments needed particularly in the "Patches" struct - which has a misleading name. I will review more the UpdateADC function once the code is a bit simplified based on the comments in this review.

shahor02
shahor02 previously approved these changes Feb 1, 2024
Copy link
Collaborator

@shahor02 shahor02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditional approval to start CI of 1st commit

@siragoni
Copy link
Contributor Author

siragoni commented Feb 1, 2024

Good evening @shahor02 , I unfortunately overwrote your approval. Could you please give it again? I think it is needed since I am actually a first time committer...

martenole
martenole previously approved these changes Feb 1, 2024
Copy link
Contributor

@martenole martenole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditional to start CI again

Copy link
Collaborator

@shahor02 shahor02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it takes some time to acknowledge the approval (done when actual build starts).

shahor02
shahor02 previously approved these changes Feb 1, 2024
Copy link
Collaborator

@shahor02 shahor02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it takes some time to update the status after the approval.

@alibuild
Copy link
Collaborator

alibuild commented Feb 2, 2024

Error while checking build/O2/fullCI for 5a90793 at 2024-02-02 05:06:

## 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/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/TRUElectronics.cxx:200:32: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/LZEROElectronics.cxx:179:31: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/LZEROElectronics.cxx:183:27: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:296:65: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:299:72: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:360:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:362:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:364:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:366:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:368:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:370:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:372:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:374:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:376:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:378:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:382:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:384:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:386:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:388:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:390:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/Detectors/EMCAL/simulation/src/DigitizerTRU.cxx:392:30: error: statement should be inside braces [readability-braces-around-statements]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:52:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:53:5: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:73:38: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:74:38: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:83:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:84:5: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:101:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:101:14: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:108:37: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:108:45: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:109:49: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:109:63: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:118:11: error: field declaration 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:119:11: error: field declaration 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
[0 more errors; see full log]

Full log here.

@mfasDa
Copy link
Collaborator

mfasDa commented Feb 2, 2024

CI errors unrelated

@siragoni
Copy link
Contributor Author

siragoni commented Feb 2, 2024

Hi everyone, just to clarify, I see that the errors are all related to statements like:

      if (foundPeakCurrentTRU)
        foundPeak = true;

Since I see that the build fails with output error: statement should be inside braces [readability-braces-around-statements] may I please ask you to confirm whether all this statements should be of the form

     if (foundPeakCurrentTRU) {
       foundPeak = true;
     }

instead? With clang-format -i of course. Thank you

@siragoni siragoni dismissed stale reviews from shahor02 and martenole via b9fc0d8 February 2, 2024 14:22
@alibuild
Copy link
Collaborator

alibuild commented Feb 2, 2024

Error while checking build/O2/fullCI for b9fc0d8 at 2024-02-02 16:15:

## 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/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:52:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:53:5: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:73:38: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:74:38: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:83:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:84:5: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:101:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:101:14: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:108:37: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:108:45: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:109:49: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:109:63: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:118:11: error: field declaration 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:119:11: error: field declaration 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:29:7: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:52:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:53:5: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:73:38: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:74:38: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:83:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:84:5: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:101:5: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:101:14: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:108:37: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:108:45: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:109:49: error: field reference 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:109:63: error: field reference 'nChanC' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:118:11: error: field declaration 'nChanA' does not match naming rule [aliceO2-member-name]
/sw/SOURCES/O2/12362-slc8_x86-64/0/DataFormats/Detectors/FIT/common/include/DataFormatsFIT/Triggers.h:119:11: error: field declaration 'nChanC' does not match naming rule [aliceO2-member-name]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

@siragoni
Copy link
Contributor Author

siragoni commented Feb 2, 2024

Good evening everyone, could you please clarify whether this is an issue on my side or something else?

@shahor02
Copy link
Collaborator

shahor02 commented Feb 2, 2024

No, it fails for the reason which should be already fixed. But given the amount of changes, better of we wait for the fullCI to rerun and become green.

@mfasDa mfasDa self-requested a review February 5, 2024 09:00
Copy link
Collaborator

@mfasDa mfasDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it is fine to merge. @shahor02 Please do a squash merge and keep ONLY the commit message of the initial commit.

@mfasDa mfasDa merged commit a8520d5 into AliceO2Group:dev Feb 5, 2024
@siragoni siragoni deleted the EMCAL-539 branch February 5, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants