FIT/FT0: new functions for FT0 to reconstruct event plane#11651
FIT/FT0: new functions for FT0 to reconstruct event plane#11651jikim1290 wants to merge 3 commits intoAliceO2Group:devfrom
Conversation
Please consider the following formatting changes to AliceO2Group#11651
| Int_t localChannelOrder[Nchannels] = { | ||
| 58, 56, 59, 57, 54, 52, 55, 53, 50, 49, 51, 48, 47, 45, 46, 44, 43, 42, 41, 40, | ||
| 61, 60, 63, 62, 14, 12, 15, 13, 10, 9, 11, 8, 7, 6, 5, 4, 39, 38, 37, 36, | ||
| 65, 64, 66, 67, 17, 16, 18, 19, 3, 2, 0, 1, 35, 34, 32, 33, | ||
| 68, 69, 70, 71, 20, 21, 22, 23, 24, 27, 25, 26, 29, 31, 28, 30, 94, 95, 92, 93, | ||
| 72, 73, 74, 75, 76, 78, 77, 79, 80, 83, 81, 82, 85, 87, 84, 86, 89, 91, 88, 90, | ||
| 173, 172, 175, 174, 206, 207, 204, 205, 169, 168, 171, 170, 202, 203, 200, 201, | ||
| 117, 116, 119, 118, 142, 143, 140, 141, 114, 112, 115, 113, 137, 139, 136, 138, | ||
| 166, 164, 167, 165, 197, 199, 196, 198, 110, 108, 111, 109, 133, 135, 132, 134, | ||
| 162, 160, 163, 161, 193, 195, 192, 194, 107, 105, 106, 104, 128, 130, 129, 131, | ||
| 159, 157, 158, 156, 188, 190, 189, 191, 99, 98, 97, 96, 120, 121, 122, 123, | ||
| 103, 102, 101, 100, 124, 125, 126, 127, 155, 153, 154, 152, 184, 186, 185, 187, | ||
| 147, 146, 145, 144, 176, 177, 178, 179, 151, 150, 149, 148, 180, 181, 182, 183}; |
There was a problem hiding this comment.
just for my curiosity , is it possible to make such parameters configurable(via key-value config or as CCDB object, etc)? Or they should be always compile-time/hardcoded?
There was a problem hiding this comment.
one could also put such a mapping in the CCDB of course. But should I merge this PR as is in any case?
There was a problem hiding this comment.
I agree with your idea for taking information from CCDB. However, this is for coming Pb-Pb data and the current analyzers are suffering from very slow CCDB connection. It can be surely improved later.
There was a problem hiding this comment.
But such CCDB request would be only once at the init stage or may be cached into file and then used several times(e.g. for "multinode" simulation mode, similar we have in anchored MC as far as I remember). Would it be still very slow in your case?
There was a problem hiding this comment.
just for my curiosity , is it possible to make such parameters configurable(via key-value config or as CCDB object, etc)? Or they should be always compile-time/hardcoded?
@afurs for the record: configurable key-value supports C-type arrays.
| // Calculate the coordinates of all the channels. | ||
| void calculateChannelCenter(); | ||
| // Get the coordinates of the center of the channel channelId. | ||
| TVector3 getChannelCenter(UInt_t channelId) { return mChannelCenter[channelId]; } |
There was a problem hiding this comment.
since this returns a copy in any case I don't think const makes a difference, but up to you of course the way you want to use this function
There was a problem hiding this comment.
I have added const for integrity of the system.
Please consider the following formatting changes to AliceO2Group#11651
Please consider the following formatting changes to AliceO2Group#11651
|
The test fails and said: sw/BUILD/QualityControl-latest/log1/31 Test #1: o2-qc-test-core .........................***Timeout 30.05 sec Is it due to problem from my side? I don't see anything wrong.. |
| 159, 157, 158, 156, 188, 190, 189, 191, 99, 98, 97, 96, 120, 121, 122, 123, | ||
| 103, 102, 101, 100, 124, 125, 126, 127, 155, 153, 154, 152, 184, 186, 185, 187, | ||
| 147, 146, 145, 144, 176, 177, 178, 179, 151, 150, 149, 148, 180, 181, 182, 183}; | ||
| ClassDefNV(Geometry, 2); |
There was a problem hiding this comment.
actually, when adding data members you should increase also the ClassDef version. The CI error is unrelated to your changes
There was a problem hiding this comment.
Thank you! I have corrected.
|
Hm but now something seems wrong since you only changed the ClassDef version, nothing more. Did a commit get lost? |
Please consider the following formatting changes to AliceO2Group#11651
I restored my update, Thank you for your nice notice |
|
Can you please give an approval to test? |
martenole
left a comment
There was a problem hiding this comment.
Approving for CI to start
new functions for FT0 to reconstruct event plane