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.

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

TPC: Update stochastic cluster and systematic track errors #1624

Merged
merged 1 commit into from
May 21, 2024

Conversation

wiechula
Copy link
Collaborator

No description provided.

Copy link

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

@chiarazampolli
Copy link
Collaborator

@shahor02 , looks ok to me, could you also take a look?

@chiarazampolli chiarazampolli added the async-2023-pbpb-apass4 Request porting to async-2023-pbpb-apass4 label May 14, 2024
@shahor02
Copy link
Collaborator

Sorry for missing your ping, fine for me. I guess @wiechula have tested it already. I'll also run some reco with it locally.

@chiarazampolli
Copy link
Collaborator

Ok, then I will wait for green light before merging.

@wiechula
Copy link
Collaborator Author

In my local test everything is propagated properly to the workflow. The final validation if not all parameters set as default are as we expect is still missing. But this is not related to this PR.

@chiarazampolli
Copy link
Collaborator

Hello @wiechula , @shahor02 ,

Did you run the final tests? Can I merge?

Chiara

@shahor02
Copy link
Collaborator

Yes, checking results now, will finish soon

@shahor02
Copy link
Collaborator

Hi,
here are the results of reco with the errors of this PR. The tracks are

Track lengths in low and high rate runs (normalized per interaction):
NTPCcl

The tracks are significantly shorter than in my old reconstruction with GPU_rec_tpc.clusterError2AdditionalY=0.1;GPU_rec_tpc.clusterError2AdditionalZ=0.15 (and no special error on seeding):
highIR_nclOldNew
I guess it is expected with the stricter errors.

We also have ~4% less matches, though it should be mostly due to the effectively stricter (due to the stricter cluster errors) matching chi2 cut (matches in the same 39kHz run, normalized per interaction):
highIR_mtcOldNew

Still, looking at the plots below, I would not change the matching chi2 (currently set to 80), since from the flattening of the matching chi2 distribution it is clear that large values are totally dominated by fakes (or badly reconstructed correct track with no use)

ITS-TPC matching (no afterburner) chi2 distribution in low and high rate runs (normalized per interaction):
matchChi2

In any case, the matches with chi2>60 contain practically no signal:
Mpipi (K0) signal (ITS-strandalone V0s removed) vs worst matching chi2 of 2 prongs in low and high rate runs (normalized per interaction):
k0vschisel

@chiarazampolli
Copy link
Collaborator

Hello @shahor02 ,

Thanks! So if I understand your comments, all is as expected. @wiechula is there something else we should check? You wanted too to run some tests, right?

Chiara

@wiechula
Copy link
Collaborator Author

@chiarazampolli , the tests were run, I'm still waiting for feedback from @miranov25

@shahor02
Copy link
Collaborator

Well, some shortening of TPC tracks is expected with strict errors but to me it seems to be too strong, also the amount of short tracks increases significantly. Better @miranov25 comments if this agrees with his optimization.

@chiarazampolli
Copy link
Collaborator

Hello,

I then propose to merge and start. Let me know if you have anything against it.

Chiara

@shahor02
Copy link
Collaborator

I would wait for response from @miranov25

@miranov25
Copy link
Contributor

Hello all.

In the scan at GSI we got 5-6 % smaller cluster track association as the error estimator is smaller - and chi2 is closer to the chi2.

In my plot below I used only tracks with ITS, but the factor in the number of clusters is similar, in my parameterization and Ruben plots

image

@chiarazampolli
Copy link
Collaborator

Hello @miranov25 ,

Please comment the plots above: I assume top is with the new cl error param, e bottom without, right?

I am now merging to move to test mode.

Chiara

@chiarazampolli chiarazampolli merged commit 721e4c7 into AliceO2Group:master May 21, 2024
7 checks passed
@miranov25
Copy link
Contributor

Hello.
Uper - is new error paramterization
middle - reference (old) parameterization
bottom - is the ratio

@miranov25
Copy link
Contributor

Hello @shahor02,

Regarding your statement about the K0s and chi2 cut, I understand your points and agree with them.

However, as we discussed over the past weeks, and especially last Wednesday, the TPC-ITS matching should be modified. I will attempt to implement a better TPC-ITS matching strategy utilizing max likelihood, taking background into account, and using checking causality for shorter TPC track (allowing them) ....
There are many aspects that can be improved, but even the initial step—adjusting the cut between the last padrow and the first allowed padrow—will significantly enhance the situation.

Marian

@shahor02
Copy link
Collaborator

I think we should try to understand why the tracks fail to use lower pad-rows and try to recover them. David provided me with the settings which can affect tracking at lower pad-rows, but it will take time to test this.

benedikt-voelkel pushed a commit that referenced this pull request May 23, 2024
@benedikt-voelkel benedikt-voelkel removed the async-2023-pbpb-apass4 Request porting to async-2023-pbpb-apass4 label Jun 17, 2024
feisenhu pushed a commit to feisenhu/O2DPG that referenced this pull request Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants