-
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
Adding plots to ITSTPC matching QC #11974
Conversation
Error while checking build/O2/fullCI for 7f8f6f1 at 2023-09-27 23:07:
Full log here. |
Moving to WIP while I fix the formatting etc. |
7f8f6f1
to
518c920
Compare
Hello @shahor02 , @noferini , @JianLIUhep, @knopers8 , @Barthelemy , |
@chiarazampolli if you keep unchanged old getters (even ie making them dummy when needed) and add new ones, the QC should not fail. Then, after merging this PR, then QC one, one can suppress old methods. |
Hello @shahor02 , |
Uploading json file to test. In order to upload, I had to change the extension, but it is a json file. |
Then the QC will be broken for a day or so. Could just add the lines below somewhere before
|
Hello @shahor02 , |
Error while checking build/O2/fullCI for 518c920 at 2023-09-29 07:52:
Full log here. |
c3d8cda
to
c9df921
Compare
Error while checking build/O2/fullCI for c3d8cda at 2023-09-29 14:26:
Full log here. |
Error while checking build/O2/fullCI for c9df921 at 2023-09-29 16:32:
Full log here. |
e45263a
to
9c079a2
Compare
Hello @shahor02 , |
Actually, the CIs fail to start with |
Error while checking build/O2/fullCI for 9c079a2 at 2023-10-02 08:02:
Full log here. |
Hello @shahor02 , @TimoWilken , |
the macOS-arm errors are genuine: to compose histo title you use more printf format specifiers than arguments:
Besides that the fullCI seg.faults in the qc-task-GLO-MTCITSTPC: I guess this is because current QC calls the dummy getters we put in this PR to not break the QC compilation. If in you local tests with modified QC it works fine, we can ignore the fullCI error. |
Hello @shahor02 , |
9c079a2
to
a31ff55
Compare
Error while checking build/O2/fullCI for a31ff55 at 2023-10-02 15:02:
Full log here. |
a31ff55
to
1c14f70
Compare
Error while checking build/O2/fullCI for 1c14f70 at 2023-10-05 02:44:
Full log here. |
Hello @shahor02 , |
But isn't there a way to avoid breaking the fullCI?
E.g. changing QC first and add a protection or simply disabling the task in the O2DPG json file in O2DPG used for the fullCI? |
Hello @martenole , |
This would require making old (obsolete) getters (used by QC) aliased to new getters instead of returning nullptr (which makes to QC crash). I don't think it is worth it: the fullCI fails where it is expected to fail in the QC and locally, with fixed QC Chiara can run fullCI (@chiarazampolli can you confirm that you run fullCI and not the FST only)
|
Hi @shahor02 ,
as reported at the beginning of the PR. I can also test the fullCI. |
Hello, fullCI needs an update in the json in O2DPG, as I planned to do after this PR. At this point, I will do it now. {code} did not work for me, something unrelated, I would say: {code} I am now opening the PR in O2DPG. Chiara |
Here it is: https://github.com/AliceO2Group/O2DPG/pull/1257/files @shahor02 , if you agree, we can merge it. |
Hello @shahor02 ,
Since the second test you proposed failed, but not due to QC, maybe we can anyway merge? Have you reviewed the PR? |
Hi @chiarazampolli , Sorry, I don't understand, why it is failing in publishing if you are using locally fixed QC? Cheers, |
Hello @shahor02 , The publishing is failing in the fullCI not in my local test. The log in #11974 (comment), is from fullCI. My local test fails in the second test you proposed, see #11974 (comment)
Cheers, Chiara |
I understand that it is the fullCI, this was the point: to run locally fullCI with local QC patch applied. If this was the case, why does it fail? |
I meant it is fullCI from the CI tests here. |
Hello @shahor02 , I am sorry to bother again, I would like to understand how we should proceed here. I repeat the situation:
Cheers, Chiara |
Hi @chiarazampolli |
Yes, it does. cheers, chiara |
Added:
@noferini
To test:
o2-global-track-cluster-reader --tpc-track-reader "tpctracks.root --reader-delay 10" --shm-segment-size 64000000000 --disable-mc --hbfutils-config o2_tfidinfo.root --track-types "ITS,TPC,ITS-TPC" --cluster-types "TPC,ITS" | o2-qc --config json:///home/zampolli/work/O2/RelVal/CPU_6/QC_GLO.json -b
Update of O2DPG json to be done.
Corresponding changes in QC: AliceO2Group/QualityControl#2001
Comparison of matching vs pt with previous version: