TOF matcher workflow re-arranged by noferini · Pull Request #6455 · AliceO2Group/AliceO2

noferini · Jun 17, 2021

This is a new PR started from a non-dev branch.
Previous one was closed (PR6423).
Missing comments by Ruben to be addressed are reported below.

What is the problem with makeConstrainedTPCTracks refit? W/o it the match cannot be used in the downstream workflows.
you could actually reserve less slots if you would count the numbers of different track types fetched by the creator, e.g. you cannot have mRecoCont->getTPCTracks().size() TPC matches, since part of TPC tracks will come via ITS-TPC or TPC-TRD etc.

BTW, what prevents processing the TPC-TRD and ITS-TPC-TRD tracks at once?

Why are you doing this check after adding the track? You could apply this trivial cut directly in the creator and simply not call the addXXXSeed at all if the track does not reach the TPC outer ref. (2 times!)

noferini · Jun 18, 2021

All comments fully addressed but:

What is the problem with makeConstrainedTPCTracks refit? W/o it the match cannot be used in the downstream workflows.
-The error is
[171553:tof-matcher]: [12:18:48][INFO] Shifting Z for 20 matched TPC tracks according to TOF time info
[171553:tof-matcher]: terminate called without an active exception
[ERROR] websocket_server_callback: Error while reading from websocket

To be defined how to split outputs for constrained track once TPC-TRD and ITS-TPC-TRD will be included

noferini · Jun 18, 2021

makeConstrainedTPCTracks problem fixes. Now it works (filling of one vector was missing)

noferini · Jun 18, 2021

build/O2/o2 check expected to be fixed after
AliceO2Group/QualityControl#735

shahor02

Thanks @noferini
Since the FullCI fails due to the QC, did you run the FST locally to make sure it is not broken.
BTW, to not forget: once this is merged, the TOF reco should be updated also in the https://github.com/AliceO2Group/O2DPG/blob/b7a46515d5bbe54c3d9cd90dd4b063f01cf7e775/MC/bin/o2dpg_sim_workflow.py#L482-L487

shahor02 · Jun 19, 2021

Detectors/GlobalTrackingWorkflow/src/TOFMatcherSpec.cxx

At the moment ok, later may change this either to more meaningful names or use subspec.

shahor02 · Jun 19, 2021

prodtests/full-system-test/dpl-workflow.sh

Now the o2-tof-reco-workflow will produce only digits and clusters, while this one will run with current defaults the TPC and ITS-TPC mathing, right?

noferini · Jun 19, 2021

Thanks @noferini
Since the FullCI fails due to the QC, did you run the FST locally to make sure it is not broken.
BTW, to not forget: once this is merged, the TOF reco should be updated also in the https://github.com/AliceO2Group/O2DPG/blob/b7a46515d5bbe54c3d9cd90dd4b063f01cf7e775/MC/bin/o2dpg_sim_workflow.py#L482-L487

Sorry, what is "FST"?
I compiled locally QC after removing the dependency and then open PR
AliceO2Group/QualityControl#735
I will fix O2DPG after the merging of this one.

shahor02 · Jun 19, 2021

Hi, FST is the prodtests/full_system_test.sh script

noferini · Jun 20, 2021

I had problem in my laptop with FST with SHM which I didn't understand but I was able to run from scratch these commands (after a minor fix in tof-reco-workflow already ported)

o2-sim-serial -n 10 -g pythia8pp -e TGeant3 -m ITS TPC TRD TOF
o2-sim-digitizer-workflow -b --onlyDet ITS,TPC,TRD,TOF --shm-segment-size 4000000000
o2-tof-reco-workflow -b --output-type clusters --shm-segment-size 4000000000
o2-its-reco-workflow -b --shm-segment-size 4000000000
o2-tpc-reco-workflow -b --shm-segment-size 4000000000 --output-type clusters,tracks
o2-tpcits-match-workflow -b --shm-segment-size 4000000000
o2-tof-matcher-workflow -b --shm-segment-size 4000000000
o2-tof-matcher-workflow -b --high-purity --shm-segment-size 4000000000
[O2/latest AEGIS/latest] ~/prodtest $>

I got proper outputs with tof-matcher-workflow (also with high-purity selected)

noferini · Jun 21, 2021

vector management (resize, reserve, ...) optimized
QC is expected to compile now
minor fix for TGeo INFO (instead of warning) ported [@davidrohr ]

noferini · Jun 22, 2021

I will re-open the PR since it seems to me that the last changes ported to QC are not included here for the tests.
Do you agree?

davidrohr · Jun 22, 2021

I don't understand what you mean by reopen?

noferini · Jun 22, 2021

I don't understand what you mean by reopen?

I see that checks are running with the old QC even if my fix is merged.
I was wondering if the new QC master will be used in the check if I close and then open a new PR

davidrohr · Jun 22, 2021

Hi Francesco, closing and reopening will not change what is used in the test. I think merging to QC master is not enough, we need a new QC tag in bump it in alidist, to test with your fix. @Barthelemy : could you take care?

Barthelemy · Jun 24, 2021

@davidrohr QC v1.20 is out, the alidist bump is pending here: alisw/alidist#3122

davidrohr · Jun 24, 2021

thx
@noferini : Could you rebase this PR to dev?

noferini · Jun 24, 2021

What is the plan for this PR?
I suspect that the conflict will continue to appear if we don't merge these changes

shahor02 · Jun 24, 2021

Hi @noferini
I'll merge it as soon as it passes the FST (will test locally once the conflicts are resolved).

davidrohr · Jun 24, 2021

Qc has been bumped, so if you rebase this it should hooefully pass the CI.

…6430) * Updated copyright headers in all files with new text agreed in the O2 project

shahor02 · Jun 24, 2021

Hi @noferini

Both the full_system_test.sh and sim_challenge are broken by this PR... I've opened a noferini#28 in your repo which fixes them: apart from workflows options/reshuffling, I've also reverted the MATCHINFO_0 and MATCHINFO_1 to MATCHINFOS_TPC and MATCHINFOS resp.. The alternative would be to modify other workflows, but this should be anyway changed on TOF side: (1) the TOF matcher should not always return all 4 MATCHINFO_X types and (2) there should be difference between the same (detector-wise) matchinfo in strict and relaxed modes.
I propose that you merge my fix, I merge this PR after the CI, then I will gradually fix the DataSpecs issue (most probably, using different subspecs).

Fixes

noferini · Jun 24, 2021

Hi @noferini

Both the full_system_test.sh and sim_challenge are broken by this PR... I've opened a noferini#28 in your repo which fixes them: apart from workflows options/reshuffling, I've also reverted the MATCHINFO_0 and MATCHINFO_1 to MATCHINFOS_TPC and MATCHINFOS resp.. The alternative would be to modify other workflows, but this should be anyway changed on TOF side: (1) the TOF matcher should not always return all 4 MATCHINFO_X types and (2) there should be difference between the same (detector-wise) matchinfo in strict and relaxed modes.
I propose that you merge my fix, I merge this PR after the CI, then I will gradually fix the DataSpecs issue (most probably, using different subspecs).

Merged in my own branch

shahor02 · Jun 25, 2021

Hi @noferini
Could you please apply from O2 dir.:

git diff --diff-filter d --name-only $(git merge-base HEAD origin/dev) | xargs sed -i 's/\t/ /g'
git diff --diff-filter d --name-only $(git merge-base HEAD origin/dev) | xargs sed -i 's/[[:space:]]*$//'

The QC v1.20 was just merged, hopefully o2 CI will be ok at next iteration.

shahor02 · Jun 26, 2021

local FST passed, merging.

* generalize TOF matcher * add MatchInfoTOFReco class * cleanup TOF reco workflow * Updated copyright headers in all files with new text … (AliceO2Group#6430) * Updated copyright headers in all files with new text agreed in the O2 project * fix copyrights new TOF classes * Fixes * fix for checks Co-authored-by: Ivana Hrivnacova <Ivana.Hrivnacova@cern.ch> Co-authored-by: shahoian <ruben.shahoyan@cern.ch>

noferini requested review from a team and shahor02 as code owners June 17, 2021 13:38

noferini force-pushed the matcher branch 5 times, most recently from b0dd5f2 to 217b3e7 Compare June 18, 2021 09:51

noferini force-pushed the matcher branch 3 times, most recently from 6b79f81 to ffe9d1b Compare June 18, 2021 14:41

noferini mentioned this pull request Jun 18, 2021

wrong TOF dependency removed AliceO2Group/QualityControl#735

Merged

shahor02 reviewed Jun 19, 2021

View reviewed changes

noferini force-pushed the matcher branch from ffe9d1b to e148260 Compare June 20, 2021 08:43

noferini force-pushed the matcher branch from e148260 to 541b46c Compare June 21, 2021 15:26

noferini force-pushed the matcher branch from 541b46c to cb8d6df Compare June 24, 2021 11:50

noferini and others added 4 commits June 24, 2021 14:00

generalize TOF matcher

2d83f1d

add MatchInfoTOFReco class

39d14cf

cleanup TOF reco workflow

d504bc1

Updated copyright headers in all files with new text … (AliceO2Group#…

e7fb9c9

…6430) * Updated copyright headers in all files with new text agreed in the O2 project

noferini force-pushed the matcher branch from cb8d6df to e7fb9c9 Compare June 24, 2021 12:03

noferini and others added 2 commits June 24, 2021 14:44

fix copyrights new TOF classes

5d05278

Fixes

5d9e0f0

Merge pull request #28 from shahor02/fix_tof_fn

0d2a405

Fixes

fix for checks

d5caa83

shahor02 merged commit 78aba6f into AliceO2Group:dev Jun 26, 2021

noferini deleted the matcher branch September 3, 2021 07:17

Search code, repositories, users, issues, pull requests...

Comments

Conversation

noferini commented Jun 17, 2021

Uh oh!

noferini commented Jun 18, 2021

Uh oh!

noferini commented Jun 18, 2021

Uh oh!

noferini commented Jun 18, 2021

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

shahor02 Jun 19, 2021

Choose a reason for hiding this comment

Uh oh!

shahor02 Jun 19, 2021

Choose a reason for hiding this comment

Uh oh!

noferini Jun 19, 2021

Choose a reason for hiding this comment

Uh oh!

noferini commented Jun 19, 2021

Uh oh!

shahor02 commented Jun 19, 2021

Uh oh!

noferini commented Jun 20, 2021

Uh oh!

noferini commented Jun 21, 2021

Uh oh!

noferini commented Jun 22, 2021

Uh oh!

davidrohr commented Jun 22, 2021

Uh oh!

noferini commented Jun 22, 2021

Uh oh!

davidrohr commented Jun 22, 2021

Uh oh!

Barthelemy commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidrohr commented Jun 24, 2021

Uh oh!

noferini commented Jun 24, 2021

Uh oh!

shahor02 commented Jun 24, 2021

Uh oh!

davidrohr commented Jun 24, 2021 via email

Uh oh!

shahor02 commented Jun 24, 2021

Uh oh!

noferini commented Jun 24, 2021

Uh oh!

shahor02 commented Jun 25, 2021

Uh oh!

shahor02 commented Jun 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

Barthelemy commented Jun 24, 2021 •

edited

Loading