Skip to content

Adding FT3 detector for ALICE3 EndCaps simulation#5526

Merged
davidrohr merged 5 commits into
AliceO2Group:devfrom
rpezzi:ALICE3_EC0
Mar 16, 2021
Merged

Adding FT3 detector for ALICE3 EndCaps simulation#5526
davidrohr merged 5 commits into
AliceO2Group:devfrom
rpezzi:ALICE3_EC0

Conversation

@rpezzi

@rpezzi rpezzi commented Feb 22, 2021

Copy link
Copy Markdown
Collaborator

This PR creates simple forward layer detector named FT3 to get started with ALICE3 endcaps simulation. This is a follow up to PR #3910. FT3 is based on the ITSMFT classes, much like the ALICE3 TRK detector. Each layer is made of a monolithic silicon disk with a thin sensitive layer for hit generation. Silicon chip thickness is tuned to match the layer x/X0 to allow a minimal evaluation of material budget effects. Detector layout follows latest Werner's presentation. Alternatively there is a function to build a parametrized detector with equidistant layers (needs recompilation to switch). Detector layout is not yet configurable without recompilation.

The detector is symmetric at forward and backward directions.

One should get a file o2sim_HitsFT3.root by running
$ o2-sim -m FT3 -e TGeant3 -g boxgen -n 10 --configKeyValues 'BoxGun.pdg=13 ; BoxGun.eta[0]=-5.0 ; BoxGun.eta[1]=5.0; BoxGun.number=500'

Created simple forward layers to get started with ALICE 3 endcaps simulation. Based on the ITSMFT classes, much like the ALICE TRK detector.

@mconcas mconcas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @rpezzi, thanks a lot for the PR. To me this looks a positive PR, just one minimal comment about some methods that seem to be not needed (I am removing their counterparts in ITS3 and TRK as well).
I am out of curiosity trying it locally, also to see how things behave in a simulation that includes new beampipe and TRK.
As soon as the fullCI is green to me this can be merged
As soon as we add also the backwards part of the detector this can be merged.

Comment thread Detectors/Upgrades/ALICE3/EC0/simulation/src/Detector.cxx Outdated
Comment thread Detectors/Upgrades/ALICE3/EC0/simulation/src/Detector.cxx Outdated
Comment thread Detectors/Upgrades/ALICE3/EC0/simulation/src/Detector.cxx Outdated
Comment thread Detectors/Upgrades/ALICE3/EC0/simulation/src/Detector.cxx Outdated
Comment thread Detectors/Upgrades/ALICE3/EC0/simulation/src/Detector.cxx
@rpezzi rpezzi changed the title Adding EC0 detector for ALICE3 EndCaps simulation Adding FT3 detector for ALICE3 EndCaps simulation Mar 1, 2021
@rpezzi

rpezzi commented Mar 1, 2021

Copy link
Copy Markdown
Collaborator Author

The detector has been named FT3 and has been mirrored with forward / backward symmetry. Removed unused methods.
This is how the detector geometry looks like

image
image

By the way, pythia events generate hits in all detectors as it should. My previous test was bogus with o2-sim default fairboxgen.

Comment thread Detectors/Upgrades/ALICE3/FT3/simulation/include/FT3Simulation/FT3Layer.h Outdated
ktf
ktf previously approved these changes Mar 2, 2021

@ktf ktf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok for the changes in DataFormats/Headers

FT3 is now composed of symmetric forward and backward layers.
This commit also removes unused methods.

@mconcas mconcas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For what concerns the Upgrades part this is fine.

@rpezzi

rpezzi commented Mar 3, 2021

Copy link
Copy Markdown
Collaborator Author

CI is happy! This PR should be good to go.
For reference, ALIBUILD_O2_TESTS=1 aliBuild ... does not run checkcode test. Instructions to run checkcode are here. Unfortunately local output was not as readable as the CI. Will keep trusting CI for codecheck.

@mconcas

mconcas commented Mar 3, 2021

Copy link
Copy Markdown
Collaborator

@sawenzel if it is ok with you on the simulation part I think we can merge, thanks.

for (int d = DetID::First; d <= DetID::Last; ++d) {
#ifdef ENABLE_UPGRADES
if (d != DetID::IT3 && d != DetID::TRK) {
if (d != DetID::IT3 && d != DetID::TRK && d != DetID::FT3) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

at some moment it will make sense to talk about a better isolation of Run3 and upgrade detectors.
Something like predefined list in separate DetIDs ..

@mconcas

mconcas commented Mar 9, 2021

Copy link
Copy Markdown
Collaborator

fullCI seems to be glitched, is there a way to re-trigger the build? @TimoWilken

@davidrohr

Copy link
Copy Markdown
Collaborator

It will rerun automatically after some time

@TimoWilken

Copy link
Copy Markdown
Contributor

@mconcas David is right, but I've marked the build yellow so it re-runs sooner.

@rpezzi

rpezzi commented Mar 10, 2021

Copy link
Copy Markdown
Collaborator Author

Hi @TimoWilken, @mconcas the error currently reported on fullCI is not related to this PR, it is on QC. Last week all CI checks run OK, just before CI broke. I wonder if one could merge this or mark the build yellow again? Thanks.

@davidrohr

Copy link
Copy Markdown
Collaborator

@Barthelemy : QC is again breaking the FullCI with

2021-03-10 03:29:39.372635     QC infologger initialized
2021-03-10 03:29:39.372700     Using config file 'json:///mnt/mesos/sandbox/sandbox/o2-fullci/sw/INSTALLROOT/214c1a6170927dfd0353c030bb72131f1fc93c71/slc8_x86-64/QualityControl/v1.11.0-1/etc/analysisDirect.json'
2021-03-10 03:29:39.372773     Creating a standalone QC topology.
2021-03-10 03:29:39.372803     Generating Data Sampling

 *** Break *** segmentation violation

The run is from this morning 3pm. Could you check? I though this was fixed?

@rpezzi

rpezzi commented Mar 10, 2021

Copy link
Copy Markdown
Collaborator Author

@davidrohr @Barthelemy , note that PRs ##5553 and #5532 ran OK this morning, both on alibuild02, while this PR ran at alientest06.

@TimoWilken

Copy link
Copy Markdown
Contributor

@Barthelemy if this issue is fixed in QC v1.12.0, should we use that here? If so, that needs to be changed in alidist. (These checks are still using v1.11.0.)

@Barthelemy

Copy link
Copy Markdown
Collaborator

It is worked around in QC v1.12 but the alidist PR is itself blocked: alisw/alidist#2887

@davidrohr

Copy link
Copy Markdown
Collaborator

@TimoWilken @Barthelemy : I am actually wondering, didn't we at some point switch to testing QualityControl/dev instead of the version in alidist?

@TimoWilken

Copy link
Copy Markdown
Contributor

@davidrohr, I think it was the other way around -- we test QC against O2@dev. The fullCI checks both currently test against the QC version from alidist (so v1.11.0 right now).

@davidrohr

Copy link
Copy Markdown
Collaborator

@davidrohr, I think it was the other way around -- we test QC against O2@dev. The fullCI checks both currently test against the QC version from alidist (so v1.11.0 right now).

Not so sure. Didn't we have the discussion in WP3 for which packets to test the dev branch? And we decided for Readout / etc. to stick to the alibuild version, but for QC we switched to dev. I actually remember seing a PR to change the QC target to dev, but I don't find it any more. Perhaps I am also remembering just wrong...

@davidrohr

Copy link
Copy Markdown
Collaborator

OK, forget about it, you were right... alisw/ali-bot#931

@Barthelemy

Copy link
Copy Markdown
Collaborator

I don't mind either ways.

@rpezzi

rpezzi commented Mar 12, 2021

Copy link
Copy Markdown
Collaborator Author

Thanks for the info. How should we proceed with this PR? Notice that it has passed CI tests on March 3. After that all failures on subsequent runs were due to bogus tests.

@mconcas

mconcas commented Mar 16, 2021

Copy link
Copy Markdown
Collaborator

@TimoWilken @davidrohr Hi guys, sorry to bother you, I know you are very busy. Do we have any update on this? Thanks in advance.

@TimoWilken

Copy link
Copy Markdown
Contributor

The fullCI error is unrelated. I've put in a PR to fix it -- #5713.

o2-dataflow is also unrelated -- the test in QC has been timing out for lots of PRs. @Barthelemy I assume this is a known problem?

@davidrohr davidrohr merged commit 2101aa7 into AliceO2Group:dev Mar 16, 2021
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.

7 participants