AliceO2 icon indicating copy to clipboard operation
AliceO2 copied to clipboard

TPC: adding cluster occupancy to timeseries

Open matthias-kleiner opened this issue 1 year ago • 16 comments

matthias-kleiner avatar Jul 05 '24 15:07 matthias-kleiner

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-2023-pp-apass4 async-2024-pp-apass1 async-2022-pp-apass7 async-2024-pp-cpass0

github-actions[bot] avatar Jul 05 '24 15:07 github-actions[bot]

Hello @miranov25 @shahor02 I tried to add the occupancy map to the time series, however when I was running it locally with

ARGS_ALL="--shm-segment-size 30000000000"
      o2-global-track-cluster-reader --disable-mc --cluster-types "TOF,TPC" --track-types "ITS,TPC,ITS-TPC,ITS-TPC-TOF,ITS-TPC-TRD-TOF" --primary-vertices --hbfutils-config o2_tfidinfo.root ${ARGS_ALL} \
      | o2-tpc-time-series-workflow --enable-unbinned-root-output --sample-unbinned-tsallis --sampling-factor 0.05  ${ARGS_ALL} -b

I also get negative values. @shahor02 do you know if something is missing in the code? occupancy

matthias-kleiner avatar Jul 05 '24 15:07 matthias-kleiner

The value should always be positive. This appears to be nonsensical—even the value itself seems off. It looks like uninitialized memory junk. I assume that the algorithm should return something, even if it isn’t initialized properly.

miranov25 avatar Jul 05 '24 15:07 miranov25

Hi @matthias-kleiner

I see that in the GPUParam internally it is interpreted as unsigned int, https://github.com/AliceO2Group/AliceO2/blob/bdb541f5d2e12861c7fb2734bffd986887fe4418/GPU/GPUTracking/Base/GPUParam.h#L62, please redefine the vector to unsigned int. However, this can hardly explain negative values. Could you check the size of vector? It should be ~32*3564/8/16.

Pinging to @davidrohr in case he sees something wrong with this way of extracting the occupancy array.

shahor02 avatar Jul 05 '24 15:07 shahor02

Hello @matthias-kleiner . Do you have a new version of the tree?

miranov25 avatar Jul 06 '24 13:07 miranov25

Hi @matthias-kleiner

I see that in the GPUParam internally it is interpreted as unsigned int,

https://github.com/AliceO2Group/AliceO2/blob/bdb541f5d2e12861c7fb2734bffd986887fe4418/GPU/GPUTracking/Base/GPUParam.h#L62 , please redefine the vector to unsigned int. However, this can hardly explain negative values. Could you check the size of vector? It should be ~32*3564/8/16.

Pinging to @davidrohr in case he sees something wrong with this way of extracting the occupancy array.

Thanks, now there are only positive values. The size of the vector is just 3576 occupancy

matthias-kleiner avatar Jul 06 '24 13:07 matthias-kleiner

Thanks, now there are only positive values. The size of the vector is just 3576

@matthias-kleiner , what data are you reading? Is it 128 HBF/orbit?

shahor02 avatar Jul 06 '24 13:07 shahor02

Hello @matthias-kleiner and @shahor02

Based on our test with Jens, I would expect to see approximately 900 values for standard settings. As Ruben suggested, it appears that you might be using the 128 HBF/orbit configuration. Are you using the pp 500 kHz data in that test?

If it is pp 500 kHz, then the occupancy aligns closely with my expectations.

Could you display the occupancy versus time? I suspect the smaller values are at the beginning and the end od the slot . I’m unsure about the spike at the end.

miranov25 avatar Jul 06 '24 14:07 miranov25

The best will be to give me the access to that particular tree

miranov25 avatar Jul 06 '24 14:07 miranov25

Here's an improved version of your message for better clarity and formality:


I attempted to open the file provided by Matthias Kleiner, but encountered some issues with reading it, which seems to be a separate problem. Inu data: (lustre/nyx/alice/users/mkleiner/test_prod/testtsPP/o2_timeseries_tpc.root. It is 2024 pp data LHC24af/550367

However, I'm unclear about what happens when you request clusters using the following code:

dataRequest->requestClusters(GID::getSourcesMask("TPC"), useMC);

We are utilizing the workflow offline , and I'm not certain if we will obtain an occupancy estimator in this context. I had assumed that occupancy is calculated during the tracking phase while reading clusters.

Is this approach feasible?

miranov25 avatar Jul 06 '24 14:07 miranov25

I was running again, but this time with the full reconstruction with the O2DPG scripts instead of loading the tracks and clusters from the local files. Now It seems more reasonable. The size is also now just 912. occupancy

matthias-kleiner avatar Jul 06 '24 15:07 matthias-kleiner

Hello @matthias-kleiner

Thank you. Good. Is this information included in the new file you provided? Still the spike at 0 is somehow surprising, so I have to check the content - after recompilation of O2.

However, this seems to be bad news. It appears that to obtain occupancy data, we must run the full reconstruction, and we cannot repeat the time series extraction without completing the entire reconstruction process.

miranov25 avatar Jul 06 '24 15:07 miranov25

The value from your plot, approximately 4.2 x 10^9, seems excessively high. This is likely due to a non-initialized array. It appears we should wait for David's input, as that number seems nonsensical.

For reference:

root [25] TMath::Power(2,32)
(double) 4.2949673e+09

miranov25 avatar Jul 06 '24 15:07 miranov25

While I'm waiting for the O2 to recompile to the latest version, could you create a histogram for only a subset of the data? Here's the command:

tree->Draw("mOccupancyMapTPC>>his(100,0,2000000)", "", "")

I'm trying to determine whether all the data are nonsensical or if only part of it is undefined.

miranov25 avatar Jul 06 '24 15:07 miranov25

We are utilizing the workflow offline , and I'm not certain if we will obtain an occupancy estimator in this context. I had assumed that occupancy is calculated during the tracking phase while reading clusters.

If you are using o2-global-track-cluster-reader for the input when running over existing reconstruction, you just need to add --cluster-types TPC to its options.

shahor02 avatar Jul 06 '24 16:07 shahor02

@matthias-kleiner @miranov25 note that the 1st bin of the array gives the integrated Nclusters, the timebins start from offset of 2: https://github.com/AliceO2Group/AliceO2/blob/91ccc94728318224e3730513eec64dd6d5b4d7b3/GPU/GPUTracking/Interface/GPUO2InterfaceRefit.cxx#L125-L130

shahor02 avatar Jul 08 '24 10:07 shahor02

@matthias-kleiner do you have the log of the test with reader? As far as I can see the code, the GRP is loaded and for me the log says at 1st TF:

[3022078:tpc-clusters-sharing-map-producer]: [18:45:55][INFO] Processing timeslice:0, tfCounter:183583, firstTForbit:33800384, runNumber:544508, creation:1697087333586, action:0
[3022078:tpc-clusters-sharing-map-producer]: [18:45:55][INFO] Caching in 128416549254992 ptr to GLO/GRPECS/0 (0x37dd540)
[3022078:tpc-clusters-sharing-map-producer]: [18:45:55][INFO] Will use 32 HB per TF from GRPECS
[3022078:tpc-clusters-sharing-map-producer]: [18:45:56][INFO] Timing for TPC clusters sharing map creation: Cpu: 6.500e-01 Real: 6.578e-01 s
[3022078:tpc-clusters-sharing-map-producer]: [18:45:56][INFO] Done processing timeslice:0, tfCounter:183583, firstTForbit:33800384, runNumber:544508, creation:1697087333586, action:0, wall:676816439

shahor02 avatar Jul 10 '24 13:07 shahor02

Hello @matthias-kleiner, @shahor02, @davidrohr,

We need the code for the apass4 production starting next week. From what we understand, the content of the histogram is reasonable, but the size has not been correctly initialized. I propose temporarily hardwiring the histogram length if this issue isn't resolved today.

This approach was already agreed upon in our discussion with Jens. Do you also agree? If so, @matthias-kleiner, could you implement this change?

We want to make similar inclusion into the AO2D, I was discussing that notion with David (Chinelatto) in mattermost.

Best regards,

Marian

miranov25 avatar Jul 11 '24 13:07 miranov25

Hello @matthias-kleiner, @shahor02, @davidrohr,

We need the code for the apass4 production starting next week. From what we understand, the content of the histogram is reasonable, but the size has not been correctly initialized. I propose temporarily hardwiring the histogram length if this issue isn't resolved today.

This approach was already agreed upon in our discussion with Jens. Do you also agree? If so, @matthias-kleiner, could you implement this change?

We want to make similar inclusion into the AO2D, I was discussing that notion with David (Chinelatto) in mattermost.

Best regards,

Marian

As I understand for the apass4 production the size will be correct, as I observed the problem with the size only when re-running the workflow with the local files. I think we can fix this also later?

matthias-kleiner avatar Jul 11 '24 13:07 matthias-kleiner

Well, as youbsay, you can easily just cut it to the 32 orbit length in the tine series workflow. I think that is the easiest solution before we have a proper fix I woukd not fix the size, but just cut it if it is longer.

davidrohr avatar Jul 11 '24 13:07 davidrohr

@matthias-kleiner , can you make fix as @davidrohr suggested?

miranov25 avatar Jul 11 '24 13:07 miranov25

@matthias-kleiner , can you make fix as @davidrohr suggested?

I can restrict it to a maximum of 912 and make it configutable. Or is there some way to calculate the size what it should be for given number of orbits per TF?

matthias-kleiner avatar Jul 11 '24 13:07 matthias-kleiner

I suggest to merge and add it to the dependencies for PBpb apass4. Can you post new file to the lustre so I can check the content?

miranov25 avatar Jul 11 '24 17:07 miranov25