-
Notifications
You must be signed in to change notification settings - Fork 433
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
FIT/FT0: new functions for FT0 to reconstruct event plane #11651
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please put const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.. |
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}; | ||
ClassDefNV(Geometry, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving for CI to start
new functions for FT0 to reconstruct event plane