AliceO2 icon indicating copy to clipboard operation
AliceO2 copied to clipboard

add ITS sensors to parallel world

Open maciacco opened this issue 1 year ago • 3 comments

@mconcas Adding ITS sensors to parallel world to prioritize navigation of sensitive elements in case of overlapping volumes

maciacco avatar May 27 '24 13:05 maciacco

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-apass3 async-2023-pbpb-apass4 async-2022-pp-apass6-2023-PbPb-apass2 async-2022-pp-apass4 async-2022-pp-apass4-accepted async-2022-pp-apass6-2023-PbPb-apass2-accepted async-2023-pbpb-apass3-accepted async-2023-pbpb-apass4-accepted async-2023-pp-apass4 async-2023-pp-apass4-accepted async-2024-pp-apass1 async-2024-pp-apass1-accepted async-2022-pp-apass7 async-2022-pp-apass7-accepted async-2024-pp-cpass0 async-2024-pp-cpass0-accepted

github-actions[bot] avatar May 27 '24 13:05 github-actions[bot]

@mconcas Adding ITS sensors to parallel world to prioritize navigation of sensitive elements in case of overlapping volumes

Thanks a lot @maciacco , approving to trigger CI.

mconcas avatar May 27 '24 13:05 mconcas

Error while checking build/O2/fullCI for d6bf6b5ce83023e330df6ecc11d4a73ee791576e at 2024-08-10 06:05:

## 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/13163-slc8_x86-64/0/Detectors/FIT/FT0/simulation/src/Detector.cxx:59:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/CPV/simulation/src/Detector.cxx:53:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/PHOS/simulation/src/Detector.cxx:61:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/TOF/simulation/src/Detector.cxx:49:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/ITSMFT/MFT/simulation/src/DigitizerTask.cxx:36:16: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/GlobalTracking/src/MatchGlobalFwd.cxx:844:17: error: use '= default' to define a trivial default constructor [modernize-use-equals-default]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

alibuild avatar Jun 06 '24 15:06 alibuild

This PR did not have any update in the last 30 days. Is it still needed? Unless further action in will be closed in 5 days.

github-actions[bot] avatar Aug 10 '24 01:08 github-actions[bot]

Error while checking build/O2/fullCI for 1b3d7e0f5d253a3e1bbefcd9d28ba3c20fe44b6e at 2024-08-15 14:01:

## 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/13163-slc8_x86-64/0/Detectors/FIT/FT0/simulation/src/Detector.cxx:59:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/CPV/simulation/src/Detector.cxx:53:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/PHOS/simulation/src/Detector.cxx:61:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/TOF/simulation/src/Detector.cxx:49:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/ITSMFT/MFT/simulation/src/DigitizerTask.cxx:36:16: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/GlobalTracking/src/MatchGlobalFwd.cxx:844:17: error: use '= default' to define a trivial default constructor [modernize-use-equals-default]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

alibuild avatar Aug 13 '24 04:08 alibuild

Error while checking build/O2/fullCI for ebe61e9dda674768b0499232e36f6bc33840726c at 2024-08-24 06:52:

## 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/13163-slc8_x86-64/0/Detectors/FIT/FT0/simulation/src/Detector.cxx:59:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/PHOS/simulation/src/Detector.cxx:61:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/CPV/simulation/src/Detector.cxx:53:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/TOF/simulation/src/Detector.cxx:49:11: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/ITSMFT/MFT/simulation/src/DigitizerTask.cxx:36:16: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13163-slc8_x86-64/0/Detectors/GlobalTracking/src/MatchGlobalFwd.cxx:844:17: error: use '= default' to define a trivial default constructor [modernize-use-equals-default]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

alibuild avatar Aug 20 '24 17:08 alibuild

Nice progress... and sorry for my late comments. I just realized that the adjustments are not done within ITS geometry code but in the global alignment framework. Not sure if there are alternatives ... but it might make sense to shortly think about if we can do it slightly better.

sawenzel avatar Aug 21 '24 14:08 sawenzel

Nice progress... and sorry for my late comments. I just realized that the adjustments are not done within ITS geometry code but in the global alignment framework. Not sure if there are alternatives ... but it might make sense to shortly think about if we can do it slightly better.

Now that you pointed this out, I realize that we can definitely do it in a more generic way. We'll re-iterate on this PR. Thanks for the review.

mconcas avatar Aug 26 '24 11:08 mconcas

Dear @sawenzel I factorized the code so that the parallel world is now filled after the alignment. This way there is no issue in the export of the geometry to file + the filling of the parallel world is handled separately by each detector (only ITS as of now) avoiding the previous clashes with the alignment framework. This required to add a new function in the base Detector class, which is overridden only for ITS. Could you please have a look to the latest version? Thank you!!

maciacco avatar Sep 09 '24 15:09 maciacco

Error while checking build/O2/fullCI for ca0a0242d1f3b3a1c7bd872f16b80598c2b1e11b at 2024-09-25 13:56:

## sw/BUILD/Rivet-latest/log
make[2]: *** [Makefile:544: core.cpp] Error 127
make[1]: *** [Makefile:440: all-recursive] Error 1
make: *** [Makefile:561: all-recursive] Error 1

Full log here.

alibuild avatar Sep 10 '24 06:09 alibuild

I will take a look at the latest restructuring soon. For now, I would like to suggest trying out the new improvements in ROOT, available in tag v6-32-04-alice2 of our ROOT (https://github.com/alisw/root/releases/tag/v6-32-04-alice2).

You need to enable these features like so:

 pw = gGeoManager->CreateParallelWorld("priority_its_sensor");
 if (getenv("TGEO_PW_USEBVH")) {
      pw->SetAccelerationMode(TGeoParallelWorld::AccelerationMode::kBVH);
    }
 if (getenv("TGEO_PW_CACHING")) {
      TGeoNavigator::SetPWSafetyCaching(true);
 }

I would be good to get an update on hit improvements, CPU runtime, memory. I personally found that adding just the ITS sensor gave the best improvements in hit numbers. Once everything confirmed, we can put this into production.

sawenzel avatar Sep 20 '24 11:09 sawenzel

Dear @sawenzel thanks for the updates and for the work on the ROOT side! We will update you soon on the simulation performance using the latest version.

maciacco avatar Sep 20 '24 11:09 maciacco