Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[EMCAL-788] EMCAL cell selection, on-the-fly calibration #11845

Merged
merged 1 commit into from
Apr 17, 2024
Merged

[EMCAL-788] EMCAL cell selection, on-the-fly calibration #11845

merged 1 commit into from
Apr 17, 2024

Conversation

mfasDa
Copy link
Collaborator

@mfasDa mfasDa commented Aug 31, 2023

  • Selection of good EMCAL cells
    • Require FEE readout (HG and LG cells)
    • Time cut around trigger peak
    • Min. cell energy
  • On-the-fly recalibration
    • Optional
    • Only in synchronous workflow

@mfasDa mfasDa requested a review from shahor02 August 31, 2023 12:36
@mfasDa mfasDa requested a review from jokonig as a code owner August 31, 2023 12:36
@mfasDa
Copy link
Collaborator Author

mfasDa commented Aug 31, 2023

Needs #11844

@mfasDa
Copy link
Collaborator Author

mfasDa commented Aug 31, 2023

@jmyrcha Please review!

@mfasDa
Copy link
Collaborator Author

mfasDa commented Aug 31, 2023

On-the-fly calibration should run in the synchronous stage while the it must not run in the asynchronous stage: In the asynchronous stage we apply the calibrations as part of the asynchronous reconstruction, consequently the event display will get fully calibrated cells and does not need to calibrate internally. In the synchronous stage we don't want to run the recalibration workflow because we want to make sure that the calibrated cells don't get mixed with the uncalibrated cells by some wildcarded data request, since we want to store uncalibrated cells persistently (in the synchronous stage we work with an a-priori calibration).

@alibuild
Copy link
Collaborator

alibuild commented Aug 31, 2023 •
edited
Loading

Error while checking build/O2/fullCI for 16ac3d6 at 2023-09-01 13:15:

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


## sw/BUILD/O2Physics-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[0 more errors; see full log]

Full log here.

shahor02
shahor02 reviewed Sep 1, 2023
View reviewed changes
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.

Fine for me, but the formatting of CMakefiles is destroyed, could you restore it?

@alibuild
Copy link
Collaborator

alibuild commented Sep 1, 2023 •
edited
Loading

Error while checking build/O2/fullCI for 9b0a4c5 at 2023-10-04 01:20:

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


## sw/BUILD/O2Physics-latest/log
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[0 more errors; see full log]

Full log here.

@mfasDa
Copy link
Collaborator Author

mfasDa commented Sep 4, 2023

I have seen that there is a way to do the rejection of low-E cells in the visualization part, so this would be optional for the producer and could be removed. Not handled however and still important is the time cut. For this I attached two plots from our QC from a run with single bunches that demonstrates the effect. Uncalibrated:
CellTimeEnergy_uncalibrated
... and calibrated:
CellTimeEnergy_calibrated
One sees here that a substantial amount of hot cells fires at time far from the trigger peak however at energies sufficiently high so that they are not removed by the energy cut.

For what concerns the on-the-fly recalibration: The amount of operations is one subtraction, one multiplication and one check for status, in each cases the coefficients are taken from an array or bitfield - the CPU overhead should be moderate. Furthermore the way it is implemented allows to switch it off via a command line argument.

@mfasDa mfasDa force-pushed the EMCAL-788 branch 2 times, most recently from eb53dd8 to a0e4311 Compare October 31, 2023 16:06
@alibuild
Copy link
Collaborator

alibuild commented Oct 31, 2023 •
edited
Loading

Error while checking build/O2/fullCI for a0e4311 at 2023-12-08 13:00:

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


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


## sw/BUILD/O2-full-system-test-latest/log
Detected critical problem in logfile reco_NOGPU.log
reco_NOGPU.log:[FATAL] error while setting up workflow in o2-eve-export-workflow: missing option: emcal-max-celltime
[FATAL] error while setting up workflow in o2-eve-export-workflow: missing option: emcal-max-celltime


## 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/9cdf24eaefed9073a479038783adabc3b5ba4b9a/slc8_x86-64/o2checkcode/1.0-local407/etc/modulefiles
++ cat
--

Full log here.

@davidrohr
Copy link
Collaborator

@mfasDa : what is the status of this PR? Is it still needed? Apparently it makes the FST crash.

@mfasDa
Copy link
Collaborator Author

mfasDa commented Dec 8, 2023

@davidrohr As can be seen above it is pending in review from @jmyrcha. Currently the event display neither uses the bad channel map nor does it reject pileup via a time cut, would be advisable to do so. As I explained above the application of the online calibration should be done in the components itself in order to not have online-calibrated cells entering the CTF.

@mfasDa
Copy link
Collaborator Author

mfasDa commented Dec 8, 2023

I don't fully understand the error message, since the option names match. The only issue I could imagine is an implicit cast issue due to the missing f in the default value specification for the new float options, which I just fixed.

@davidrohr
Copy link
Collaborator

@mfasDa : could you revert the CMake formatting changes, and only add your change?
Or if the formatting change came from running the O2 CMaker formatter, it should go into a seperate commit.

@jmyrcha : Could you check if you have objections? Otherwise I'll merge it once formatting changes are fixed.

@alibuild
Copy link
Collaborator

alibuild commented Dec 15, 2023 •
edited
Loading

Error while checking build/O2/fullCI for e9e9f3e at 2023-12-16 12:28:

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


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


## sw/BUILD/O2-full-system-test-latest/log
command /sw/slc8_x86-64/O2/11845-slc8_x86-64-local28/prodtests/full-system-test/dpl-workflow.sh had nonzero exit code 1


## 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/c49f6c2ae9ebcfcd629ef3506bbd49ff84f34265/slc8_x86-64/o2checkcode/1.0-local514/etc/modulefiles
++ cat
--

Full log here.

@mfasDa
[EMCAL-788] EMCAL cell selection, on-the-fly calibration
47427fd 
- Seletion of good EMCAL cells
  - Require FEE readout (HG and LG cells)
  - Time cut around trigger peak
  - Min. cell energy
- On-the-fly recalibration
  - Optional
  - Only in synchronous workflow
@mfasDa
Copy link
Collaborator Author

mfasDa commented Dec 18, 2023

Hi @davidrohr, indeed my VSCode autoformat settings applied the cmake formatter when saving the file after changes. I manually unapplied now all the formatting changes in the CMakeLists.txt - now there are only the dependencies left over which I added.

@alibuild
Copy link
Collaborator

alibuild commented Jan 12, 2024 •
edited
Loading

Error while checking build/O2/fullCI for 47427fd at 2024-04-10 17:43:

## 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/11845-slc8_x86-64/0/Framework/Core/include/Framework/ASoA.h:3115:81: error: invalid use of incomplete type 'typename table_t::RowViewFiltered<Filtered<Join<TracksExtra_001Extension, Table<TPCInnerParam, Flags, ITSClusterSizes, TPCNClsFindable, TPCNClsFindableMinusFound, TPCNClsFindableMinusCrossedRows, TPCNClsShared, TRDPattern, ITSChi2NCl, TPCChi2NCl, TRDChi2, TOFChi2, TPCSignal, TRDSignal, Length, TOFExpMom, PIDForTracking<Flags>, IsPVContributor<Flags>, HasITS<DetectorMap>, HasTPC<DetectorMap>, HasTRD<DetectorMap>, HasTOF<DetectorMap>, TPCNClsFound<TPCNClsFindable, TPCNClsFindableMinusFound>, TPCNClsCrossedRows<TPCNClsFindable, TPCNClsFindableMinusCrossedRows>, ITSClusterMap<ITSClusterSizes>, ITSNCls<ITSClusterSizes>, ITSNClsInnerBarrel<ITSClusterSizes>, ITSClsSizeInLayer<ITSClusterSizes>, TPCCrossedRowsOverFindableCls<TPCNClsFindable, TPCNClsFindableMinusCrossedRows>, TPCFoundOverFindableCls<TPCNClsFindable, TPCNClsFindableMinusFound>, TPCFractionSharedCls<TPCNClsShared, TPCNClsFindable, TPCNClsFindableMinusFound>, TrackEtaEMCAL, TrackPhiEMCAL, TrackTime, TrackTimeRes>>>, TracksExtra_001Extension, Table<TPCInnerParam, Flags, ITSClusterSizes, TPCNClsFindable, TPCNClsFindableMinusFound, TPCNClsFindableMinusCrossedRows, TPCNClsShared, TRDPattern, ITSChi2NCl, TPCChi2NCl, TRDChi2, TOFChi2, TPCSignal, TRDSignal, Length, TOFExpMom, PIDForTracking<Flags>, IsPVContributor<Flags>, HasITS<DetectorMap>, HasTPC<DetectorMap>, HasTRD<DetectorMap>, HasTOF<DetectorMap>, TPCNClsFound<TPCNClsFindable, TPCNClsFindableMinusFound>, TPCNClsCrossedRows<TPCNClsFindable, TPCNClsFindableMinusCrossedRows>, ITSClusterMap<ITSClusterSizes>, ITSNCls<ITSClusterSizes>, ITSNClsInnerBarrel<ITSClusterSizes>, ITSClsSizeInLayer<ITSClusterSizes>, TPCCrossedRowsOverFindableCls<TPCNClsFindable, TPCNClsFindableMinusCrossedRows>, TPCFoundOverFindableCls<TPCNClsFindable, TPCNClsFindableMinusFound>, TPCFractionSharedCls<TPCNClsShared, TPCNClsFindable, TPCNClsFindableMinusFound>, TrackEtaEMCAL, TrackPhiEMCAL, TrackTime, TrackTimeRes>>' (aka 'RowViewBase<o2::soa::FilteredIndexPolicy, o2::soa::Filtered<o2::soa::Join<o2::aod::TracksExtra_001Extension, o2::soa::Table<o2::aod::track::TPCInnerParam, o2::aod::track::Flags, o2::aod::track::ITSClusterSizes, o2::aod::track::TPCNClsFindable, o2::aod::track::TPCNClsFindableMinusFound, o2::aod::track::TPCNClsFindableMinusCrossedRows, o2::aod::track::TPCNClsShared, o2::aod::track::TRDPattern, o2::aod::track::ITSChi2NCl, o2::aod::track::TPCChi2NCl, o2::aod::track::TRDChi2, o2::aod::track::TOFChi2, o2::aod::track::TPCSignal, o2::aod::track::TRDSignal, o2::aod::track::Length, o2::aod::track::TOFExpMom, o2::aod::track::PIDForTracking<o2::aod::track::Flags>, o2::aod::track::IsPVContributor<o2::aod::track::Flags>, o2::aod::track::HasITS<o2::aod::track::v001::DetectorMap>, o2::aod::track::HasTPC<o2::aod::track::v001::DetectorMap>, o2::aod::track::HasTRD<o2::aod::track::v001::DetectorMap>, o2::aod::track::HasTOF<o2::aod::track::v001::DetectorMap>, o2::aod::track::TPCNClsFound<o2::aod::track::TPCNClsFindable, o2::aod::track::TPCNClsFindableMinusFound>, o2::aod::track::TPCNClsCrossedRows<o2::aod::track::TPCNClsFindable, o2::aod::track::TPCNClsFindableMinusCrossedRows>, o2::aod::track::v001::ITSClusterMap<o2::aod::track::ITSClusterSizes>, o2::aod::track::v001::ITSNCls<o2::aod::track::ITSClusterSizes>, o2::aod::track::v001::ITSNClsInnerBarrel<o2::aod::track::ITSClusterSizes>, o2::aod::track::v001::ITSClsSizeInLayer<o2::aod::track::ITSClusterSizes>, o2::aod::track::TPCCrossedRowsOverFindableCls<o2::aod::track::TPCNClsFindable, o2::aod::track::TPCNClsFindableMinusCrossedRows>, o2::aod::track::TPCFoundOverFindableCls<o2::aod::track::TPCNClsFindable, o2::aod::track::TPCNClsFindableMinusFound>, o2::aod::track::TPCFractionSharedCls<o2::aod::track::TPCNClsShared, o2::aod::track::TPCNClsFindable, o2::aod::track::TPCNClsFindableMinusFound>, o2::aod::track::TrackEtaEMCAL, o2::aod::track::TrackPhiEMCAL, o2::aod::track::TrackTime, o2::aod::track::TrackTimeRes>>>, o2::aod::TracksExtra_001Extension, o2::soa::Table<o2::aod::track::TPCInne/sw/SOURCES/O2/11845-slc8_x86-64/0/Framework/Core/include/Framework/ASoA.h:1401:114: note: in instantiation of template class 'o2::soa::Filtered<o2::soa::Join<aod::Tracks, aod::TracksExtra>>' requested here
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

@davidrohr
Copy link
Collaborator

@mfasDa : Sorry, I failed to follow it up. Now it is green, and the formatting seems ok.
Shall we merge it?

@mfasDa
Copy link
Collaborator Author

mfasDa commented Apr 17, 2024

@davidrohr , yes I think we should merge it. The requested review will probably never come :(

@davidrohr davidrohr merged commit 94eca0d into AliceO2Group:dev Apr 17, 2024
12 checks passed
@mfasDa mfasDa deleted the EMCAL-788 branch April 17, 2024 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahor02 shahor02 shahor02 left review comments

@jokonig jokonig Awaiting requested review from jokonig

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mfasDa @alibuild @davidrohr @shahor02