TPC: Derivative map implementation #11585

ChSonnabend · 2023-06-29T14:06:10Z

This PR intends to bring the functionality to use the reference or derivative map for the SC distortion corrections depending on a flag that is specified by the user.

ChSonnabend · 2023-06-29T14:06:27Z

Pinging @wiechula

Please consider the following formatting changes to AliceO2Group#11585

GPU/TPCFastTransformation/TPCFastTransform.h

Detectors/TPC/calibration/src/CorrectionMapsLoader.cxx

Please consider the following formatting changes to AliceO2Group#11585

wiechula

@ChSonnabend , a few more comments, please mark each as solved once done.

Detectors/TPC/base/include/TPCBase/CDBInterface.h

Detectors/TPC/calibration/src/CorrectionMapsLoader.cxx

GPU/TPCFastTransformation/CorrectionMapsHelper.h

DataFormats/Detectors/CTP/include/DataFormatsCTP/LumiInfo.h

Please consider the following formatting changes to AliceO2Group#11585

wiechula

A few more things that should be fixed.

DataFormats/Detectors/CTP/include/DataFormatsCTP/LumiInfo.h

Detectors/TPC/base/include/TPCBase/CDBInterface.h

Detectors/TPC/workflow/src/tpc-reco-workflow.cxx

GPU/Workflow/src/GPUWorkflowSpec.cxx

GPU/TPCFastTransformation/CorrectionMapsHelper.h

wiechula · 2023-07-05T11:09:43Z

Detectors/TPC/base/include/TPCBase/CDBInterface.h

  {CDBType::CalCorrMap, "TPC/Calib/CorrectionMap"},
  {CDBType::CalCorrMapRef, "TPC/Calib/CorrectionMapRef"},
+  // derivative map correction
+  {CDBType::CalCorrDerivMap, "TPC/Calib/CorrectionDerivativeMap"},


Suggested change

{CDBType::CalCorrDerivMap, "TPC/Calib/CorrectionDerivativeMap"},

{CDBType::CalCorrDerivMap, "TPC/Calib/CorrectionMapDerivative"},

wiechula · 2023-07-05T11:12:21Z

GPU/TPCFastTransformation/CorrectionMapsHelper.cxx

  mUpdatedFlags = 0;
  mInstLumi = 0.f;
  mMeanLumi = 0.f;
+  mLumiScaleMode = 0;


Would this change the lumi mode during run time?

Suggested change

mLumiScaleMode = 0;

GPU/TPCFastTransformation/CorrectionMapsHelper.h

wiechula

Ok, looks fine so far I think. I converted it to a draft until we tested it.

wiechula

Thanks! Looks fine to me.

davidrohr · 2023-07-12T19:36:44Z

let's start the CI then :)

wiechula · 2023-07-13T07:01:15Z

The errors look unrelated.

martenole · 2023-07-14T17:27:23Z

Hi @ChSonnabend and @davidrohr
this PR breaks my local CTF reprocessing. I get the following errors:

[2819465:gpu-reconstruction]: Error in <TBufferFile::ReadClassBuffer>: Could not find the StreamerInfo for version 3 of the class o2::gpu::TPCFastTransform, object skipped at offset 83
[2819465:gpu-reconstruction]: Error in <TBufferFile::CheckByteCount>: object of class o2::gpu::TPCFastTransform read too few bytes: 2 instead of 158030113

followed by a segfault of the GPU reconstruction.

Any ideas?

martenole · 2023-07-14T17:39:55Z

Looking at slide 8 of https://docs.google.com/presentation/d/1WPNixo57zAn_gM6vMCIBOHs-DZqcKAkLX0CwUgO_VYg/edit#slide=id.g123dba8cd3d_0_0 it seems the schema evolution is not working and all objects from TPC/Calib/CorrectionMap and TPC/Calib/CorrectionMapRef cannot be read anymore, can this be?

shahor02 · 2023-07-14T18:03:17Z

Corrmap is a FlatObject, it is not streamed by the root but is cast from the memory block, so it has no schema evolution

davidrohr · 2023-07-14T18:08:52Z

Weird, why was this not seen by the CI? Kind Regards David Rohr Sent from my mobile. (Excuse the typos!)

…

On 14 July 2023 19:27:35 CEST, Ole Schmidt ***@***.***> wrote: Hi @ChSonnabend and @davidrohr this PR breaks my local CTF reprocessing. I get the following errors: ``` [2819465:gpu-reconstruction]: Error in <TBufferFile::ReadClassBuffer>: Could not find the StreamerInfo for version 3 of the class o2::gpu::TPCFastTransform, object skipped at offset 83 [2819465:gpu-reconstruction]: Error in <TBufferFile::CheckByteCount>: object of class o2::gpu::TPCFastTransform read too few bytes: 2 instead of 158030113 ``` followed by a segfault of the GPU reconstruction. Any ideas? -- Reply to this email directly or view it on GitHub: #11585 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

shahor02 · 2023-07-14T18:23:33Z

Haven't we discussed that with the change of the object layout we should introduce new CCDB paths? On Jul 14, 2023 20:40, Ole Schmidt ***@***.***> wrote: Looking at slide 8 of https://docs.google.com/presentation/d/1WPNixo57zAn_gM6vMCIBOHs-DZqcKAkLX0CwUgO_VYg/edit#slide=id.g123dba8cd3d_0_0 it seems the schema evolution is not working and all objects from TPC/Calib/CorrectionMap and TPC/Calib/CorrectionMapRef cannot be read anymore, can this be? — Reply to this email directly, view it on GitHub<#11585 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABYKIDL3BEZ7QKM2L7UTDS3XQF77LANCNFSM6AAAAAAZYSESVA>. You are receiving this because you commented.Message ID: ***@***.***>

martenole · 2023-07-14T19:17:14Z

Weird, why was this not seen by the CI? Kind Regards David Rohr Sent from my mobile. (Excuse the typos!)
…
On 14 July 2023 19:27:35 CEST, Ole Schmidt @.> wrote: Hi @ChSonnabend and @davidrohr this PR breaks my local CTF reprocessing. I get the following errors: [2819465:gpu-reconstruction]: Error in <TBufferFile::ReadClassBuffer>: Could not find the StreamerInfo for version 3 of the class o2::gpu::TPCFastTransform, object skipped at offset 83 [2819465:gpu-reconstruction]: Error in <TBufferFile::CheckByteCount>: object of class o2::gpu::TPCFastTransform read too few bytes: 2 instead of 158030113 followed by a segfault of the GPU reconstruction. Any ideas? -- Reply to this email directly or view it on GitHub: #11585 (comment) You are receiving this because you were mentioned. Message ID: @.>

I am not sure. But in the CI the ideal correction map is loaded, a dummy object. When I reconstruct some recent run then the analytical map is used and that fails.
Should we revert for now? The FST seems fine, but if we want to run with the new O2 version I just installed on the EPNs then we use the analytical maps and this I fear will not work. So we would need to upload new obects with fixed class version.
If in any case we should use a new CCDB path I think it would be better to revert this PR and then do another iteration with the changes and simultaneously fix the CCDB path.
Likely also others have problems when they try to re-reconstruct some data using O2@dev at the moment

This reverts commit 939bab9.

martenole · 2023-07-14T19:19:30Z

PR with the revert is here: #11658
Let me know what you think @davidrohr or @shahor02

shahor02 · 2023-07-14T19:41:15Z

If we don't have the maps in the new layout to upload to alternative ccdb path together with PR pointing to it, then it is fine to revert it.

ChSonnabend · 2023-07-14T20:16:53Z

This is something that only @wiechula can answer to; However, I think they are not available currently.

* Revert "TPC: Derivative map implementation (#11585)" This reverts commit 939bab9. * Clang format

* Adding option for using derivative map * Fixing formatting * Please consider the following formatting changes * Flag passing from the tpc-reco-workflow to gpu configs * Please consider the following formatting changes * Setting path to derivative map CCDB objects * Please consider the following formatting changes * Removing unnecessary CDB element for derivative map * Please consider the following formatting changes * Updating workflow and removing obsolete functions * Setting mLumiScaleMode at loading of CCDB object * [BugFix] Fixing accidental deletion of function --------- Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

…ceO2Group#11658) * Revert "TPC: Derivative map implementation (AliceO2Group#11585)" This reverts commit 939bab9. * Clang format

Adding option for using derivative map

6f0452c

ChSonnabend and others added 3 commits June 29, 2023 16:33

Fixing formatting

03cf492

Please consider the following formatting changes

08a7da7

Merge pull request #1 from alibuild/alibot-cleanup-11585

a7beabd

Please consider the following formatting changes to AliceO2Group#11585

wiechula requested changes Jun 30, 2023

View reviewed changes

ChSonnabend and others added 6 commits June 30, 2023 19:10

Flag passing from the tpc-reco-workflow to gpu configs

4ad7708

Please consider the following formatting changes

72a0a79

Merge pull request #2 from alibuild/alibot-cleanup-11585

35080c4

Please consider the following formatting changes to AliceO2Group#11585

Setting path to derivative map CCDB objects

919c20d

Please consider the following formatting changes

ab20d67

Merge pull request #3 from alibuild/alibot-cleanup-11585

85e8c4c

Please consider the following formatting changes to AliceO2Group#11585

ChSonnabend marked this pull request as ready for review July 1, 2023 18:50

ChSonnabend requested review from davidrohr and sgorbuno as code owners July 1, 2023 18:50

wiechula requested changes Jul 3, 2023

View reviewed changes

ChSonnabend and others added 3 commits July 3, 2023 15:48

Removing unnecessary CDB element for derivative map

b59a4e9

Please consider the following formatting changes

348cda1

Merge pull request #4 from alibuild/alibot-cleanup-11585

d8034df

Please consider the following formatting changes to AliceO2Group#11585

wiechula requested changes Jul 3, 2023

View reviewed changes

Updating workflow and removing obsolete functions

c77aa26

wiechula changed the title ~~Derivative map implementation~~ TPC: Derivative map implementation Jul 5, 2023

wiechula requested changes Jul 5, 2023

View reviewed changes

Setting mLumiScaleMode at loading of CCDB object

b9497f3

wiechula marked this pull request as draft July 6, 2023 11:38

wiechula reviewed Jul 6, 2023

View reviewed changes

ChSonnabend added 2 commits July 12, 2023 13:45

Merge branch 'dev' into derivative_map

198a6ba

[BugFix] Fixing accidental deletion of function

58bb88e

wiechula approved these changes Jul 12, 2023

View reviewed changes

davidrohr marked this pull request as ready for review July 12, 2023 19:36

davidrohr approved these changes Jul 12, 2023

View reviewed changes

davidrohr merged commit 939bab9 into AliceO2Group:dev Jul 13, 2023

martenole added a commit to martenole/AliceO2 that referenced this pull request Jul 14, 2023

Revert "TPC: Derivative map implementation (AliceO2Group#11585)"

9419db6

This reverts commit 939bab9.

martenole added a commit that referenced this pull request Jul 14, 2023

Revert "TPC: Derivative map implementation (#11585)" (#11658)

1b141a2

* Revert "TPC: Derivative map implementation (#11585)" This reverts commit 939bab9. * Clang format

ChSonnabend deleted the derivative_map branch July 23, 2023 18:42

	{CDBType::CalCorrDerivMap, "TPC/Calib/CorrectionDerivativeMap"},
	{CDBType::CalCorrDerivMap, "TPC/Calib/CorrectionMapDerivative"},

TPC: Derivative map implementation #11585

TPC: Derivative map implementation #11585

Uh oh!

Conversation

ChSonnabend commented Jun 29, 2023

Uh oh!

ChSonnabend commented Jun 29, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wiechula left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wiechula left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wiechula Jul 5, 2023

Choose a reason for hiding this comment

Uh oh!

wiechula Jul 5, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wiechula left a comment

Choose a reason for hiding this comment

Uh oh!

wiechula left a comment

Choose a reason for hiding this comment

Uh oh!

davidrohr commented Jul 12, 2023

Uh oh!

wiechula commented Jul 13, 2023

Uh oh!

martenole commented Jul 14, 2023

Uh oh!

martenole commented Jul 14, 2023

Uh oh!

shahor02 commented Jul 14, 2023

Uh oh!

davidrohr commented Jul 14, 2023 via email

Uh oh!

shahor02 commented Jul 14, 2023 via email

Uh oh!

martenole commented Jul 14, 2023

Uh oh!

martenole commented Jul 14, 2023

Uh oh!

shahor02 commented Jul 14, 2023

Uh oh!

ChSonnabend commented Jul 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants